yellow team mailing list archive
-
yellow team
-
Mailing list archive
-
Message #01232
[Merge] lp:~gary/juju-gui/simplifycharmstore into lp:juju-gui
Gary Poster has proposed merging lp:~gary/juju-gui/simplifycharmstore into lp:juju-gui.
Requested reviews:
Juju GUI Hackers (juju-gui)
For more details, see:
https://code.launchpad.net/~gary/juju-gui/simplifycharmstore/+merge/131086
Last change round of charm store data structures
This change is hopefully the last round of changes, at least for a long while, to the underlying charm store infrastructure. It is more deletes than additions, and changes the code to take advantage of the changes Kapil made to the charm store.
I made some decisions as to how to factor some of these things and would be happy to describe my rationale for changes if desired. I'm thinking particularly of the immediate creation of charm objects as search results. That allowed for some simplifications and I think is mostly a win.
The sorting code is simplified yet again.
Thanks
Gary
https://codereview.appspot.com/6733067/
--
https://code.launchpad.net/~gary/juju-gui/simplifycharmstore/+merge/131086
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~gary/juju-gui/simplifycharmstore into lp:juju-gui.
=== modified file 'app/app.js'
--- app/app.js 2012-10-19 01:44:45 +0000
+++ app/app.js 2012-10-23 20:09:21 +0000
@@ -261,7 +261,8 @@
show_unit: function(req) {
console.log(
'App: Route: Unit', req.params.id, req.path, req.pendingRoutes);
- var unit_id = req.params.id.replace('-', '/');
+ // This replacement honors service names that have a hyphen in them.
+ var unit_id = req.params.id.replace(/^(\S+)-(\d+)$/, '$1/$2');
var unit = this.db.units.getById(unit_id);
if (unit) {
// Once the unit is loaded we need to get the full details of the
=== modified file 'app/models/charm.js'
--- app/models/charm.js 2012-10-19 01:53:03 +0000
+++ app/models/charm.js 2012-10-23 20:09:21 +0000
@@ -2,67 +2,12 @@
YUI.add('juju-charm-models', function(Y) {
+
var models = Y.namespace('juju.models');
- // This is how the charm_id_re regex works for various inputs. The first
- // element is always the initial string, which we have elided in the
- // examples.
- // 'cs:~marcoceppi/precise/word-press-17' ->
- // [..."cs", "marcoceppi", "precise", "word-press", "17"]
- // 'cs:~marcoceppi/precise/word-press' ->
- // [..."cs", "marcoceppi", "precise", "word-press", undefined]
- // 'cs:precise/word-press' ->
- // [..."cs", undefined, "precise", "word-press", undefined]
- // 'cs:precise/word-press-17'
- // [..."cs", undefined, "precise", "word-press", "17"]
- var charm_id_re = /^(?:(\w+):)?(?:~(\S+)\/)?(\w+)\/(\S+?)(?:-(\d+))?$/,
- parse_charm_id = function(id) {
- var parts = charm_id_re.exec(id),
- result = {};
- if (parts) {
- parts.shift();
- Y.each(
- Y.Array.zip(
- ['scheme', 'owner', 'series', 'package_name', 'revision'],
- parts),
- function(pair) { result[pair[0]] = pair[1]; });
- if (!Y.Lang.isValue(result.scheme)) {
- result.scheme = 'cs'; // This is the default.
- }
- return result;
- }
- // return undefined;
- },
- _calculate_full_charm_name = function(elements) {
- var tmp = [elements.series, elements.package_name];
- if (elements.owner) {
- tmp.unshift('~' + elements.owner);
- }
- return tmp.join('/');
- },
- _calculate_charm_store_path = function(elements) {
- return [(elements.owner ? '~' + elements.owner : 'charms'),
- elements.series, elements.package_name, 'json'].join('/');
- },
- _calculate_base_charm_id = function(elements) {
- return elements.scheme + ':' + _calculate_full_charm_name(elements);
- },
- _reconsititute_charm_id = function(elements) {
- return _calculate_base_charm_id(elements) + '-' + elements.revision;
- },
- _clean_charm_data = function(data) {
- data.is_subordinate = data.subordinate;
- Y.each(['subordinate', 'name', 'revision', 'store_revision'],
- function(nm) { delete data[nm]; });
- return data;
- };
- // This is exposed for testing purposes.
- models.parse_charm_id = parse_charm_id;
-
- // For simplicity and uniformity, there is a single Charm class and a
- // single CharmList class. Charms, once instantiated and loaded with data
- // from their respective sources, are immutable and read-only. This reflects
- // the reality of how we interact with them.
+ // Charms, once instantiated and loaded with data from their respective
+ // sources, are immutable and read-only. This reflects the reality of how we
+ // interact with them.
// Charm instances can represent both environment charms and charm store
// charms. A charm id is reliably and uniquely associated with a given
@@ -70,127 +15,155 @@
// Therefore, the database keeps these charms separate in two different
// CharmList instances. One is db.charms, representing the environment
- // charms. The other is maintained by and within the persistent charm panel
- // instance. As you'd expect, environment charms are what to use when
- // viewing or manipulating the environment. Charm store charms are what we
- // can browse to select and deploy new charms to the environment.
+ // charms. The other, from the charm store, is maintained by and within the
+ // persistent charm panel instance. As you'd expect, environment charms are
+ // what to use when viewing or manipulating the environment. Charm store
+ // charms are what we can browse to select and deploy new charms to the
+ // environment.
- // Environment charms begin their lives with full charm ids, as provided by
- // services in the environment:
+ // Charms begin their lives with full charm ids, as provided by
+ // services in the environment and the charm store:
// [SCHEME]:(~[OWNER]/)?[SERIES]/[PACKAGE NAME]-[REVISION].
// With an id, we can instantiate a charm: typically we use
- // "db.charms.add({id: [ID]})". Finally, we load the charm's data from the
- // environment using the standard YUI Model method "load," providing an
- // object with a get_charm callable, and an optional callback (see YUI
- // docs). The env has a get_charm method, so, by design, it works nicely:
- // "charm.load(env, optionalCallback)". The get_charm method is expected to
- // return what the env version does: either an object with a "result" object
- // containing the charm data, or an object with an "err" attribute.
-
- // The charms in the charm store have a significant difference, beyond the
- // source of their data: they are addressed in the charm store by a path
- // that does not include the revision number, and charm store searches do
- // not include revision numbers in the results. Therefore, we cannot
- // immediately instantiate a charm, because it requires a full id in order
- // to maintain the idea of an immutable charm associated with a unique charm
- // id. However, the charm information that returns does have a revision
- // number (the most recent); moreover, over time the charm may be updated,
- // leading to a new charm revision. We model this by creating a new charm.
-
- // Since we cannot create or search for charms without a revision number
- // using the normal methods, the charm list has a couple of helpers for this
- // story. The workhorse is "loadOneByBaseId". A "base id" is an id without
- // a revision.
-
- // The arguments to "loadOneById" are a base id and a hash of other options.
- // The hash must have a "charm_store" attribute, that itself loadByPath
- // method, like the one in app/store/charm.js. It may have zero or more of
- // the following: a success callback, accepting the fully loaded charm with
- // the newest revision for the given base id; a failure callback, accepting
- // the Y.io response object after a failure; and a "force" attribute that,
- // if it is a Javascript boolean truth-y value, forces a load even if a
- // charm with the given id already is in the charm list.
-
- // "getOneByBaseId" simply returns the charm with the highest revision and
- // "the given base id from the charm list, without trying to load
- // "information.
+ // "db.charms.add({id: [ID]})". Finally, we load the charm's data over the
+ // network using the standard YUI Model method "load," providing an object
+ // with a get_charm callable, and an optional callback (see YUI docs). Both
+ // the env and the charm store have a get_charm method, so, by design, it
+ // works easily: "charm.load(env, optionalCallback)" or
+ // "charm.load(charm_store, optionalCallback)". The get_charm method must
+ // either callback using the default YUI approach for this code, a boolean
+ // indicating failure, and a result; or it must return what the env version
+ // does: an object with a "result" object containing the charm data, or an
+ // object with an "err" attribute.
// In both cases, environment charms and charm store charms, a charm's
// "loaded" attribute is set to true once it has all the data from its
// environment.
- var Charm = Y.Base.create('charm', Y.Model, [], {
- initializer: function() {
- this.loaded = false;
- this.on('load', function() { this.loaded = true; });
- },
- sync: function(action, options, callback) {
- if (action !== 'read') {
- throw (
- 'Only use the "read" action; "' + action + '" not supported.');
- }
- if (!Y.Lang.isValue(options.get_charm)) {
- throw 'You must supply a get_charm function.';
- }
- options.get_charm(
- this.get('id'),
- // This is the success callback, or the catch-all callback for
- // get_charm.
- function(response) {
- // Handle the env.get_charm response specially, for ease of use. If
- // it doesn't match that pattern, pass it through.
- if (response.err) {
- callback(true, response);
- } else if (response.result) {
- callback(false, response.result);
- } else { // This would typically be a string.
- callback(false, response);
+ var charm_id_re = /^(?:(\w+):)?(?:~(\S+)\/)?(\w+)\/(\S+?)-(\d+)$/,
+ id_elements = ['scheme', 'owner', 'series', 'package_name', 'revision'],
+ Charm = Y.Base.create('charm', Y.Model, [], {
+ initializer: function() {
+ var id = this.get('id'),
+ parts = id && charm_id_re.exec(id),
+ self = this;
+ if (!Y.Lang.isValue(id) || !parts) {
+ throw 'Developers must initialize charms with a well-formed id.';
+ }
+ this.loaded = false;
+ this.on('load', function() { this.loaded = true; });
+ parts.shift();
+ Y.each(
+ Y.Array.zip(id_elements, parts),
+ function(pair) { self.set(pair[0], pair[1]); });
+ // full_name
+ var tmp = [this.get('series'), this.get('package_name')],
+ owner = this.get('owner');
+ if (owner) {
+ tmp.unshift('~' + owner);
+ }
+ this.set('full_name', tmp.join('/'));
+ // charm_store_path
+ this.set(
+ 'charm_store_path',
+ [(owner ? '~' + owner : 'charms'),
+ this.get('series'),
+ (this.get('package_name') + '-' + this.get('revision')),
+ 'json'
+ ].join('/'));
+ },
+ sync: function(action, options, callback) {
+ if (action !== 'read') {
+ throw (
+ 'Only use the "read" action; "' + action + '" not supported.');
+ }
+ if (Y.Lang.isValue(options.get_charm)) {
+ // This is an env.
+ options.get_charm(
+ this.get('id'),
+ function(response) {
+ if (response.err) {
+ callback(true, response);
+ } else if (response.result) {
+ callback(false, response.result);
+ } else {
+ // What's going on? This does not look like either of our
+ // expected signatures. Declare a loading error.
+ callback(true, response);
+ }
+ }
+ );
+ } else if (Y.Lang.isValue(options.loadByPath)) {
+ // This is a charm store.
+ options.loadByPath(
+ this.get('charm_store_path'),
+ { success: function(response) {
+ callback(false, response);
+ },
+ failure: function(response) {
+ callback(true, response);
+ }
+ });
+ } else {
+ throw 'You must supply a get_charm or loadByPath function.';
+ }
+ },
+ parse: function() {
+ var data = Charm.superclass.parse.apply(this, arguments),
+ self = this;
+ data.is_subordinate = data.subordinate;
+ Y.each(data, function(value, key) {
+ if (!value ||
+ !self.attrAdded(key) ||
+ Y.Lang.isValue(self.get(key))) {
+ delete data[key];
}
- },
- // This is the optional error callback.
- function(response) {
- callback(true, response);
- }
- );
- },
- parse: function() {
- return _clean_charm_data(Charm.superclass.parse.apply(this, arguments));
- }
- }, {
- ATTRS: {
- id: {
- lazyAdd: false,
- setter: function(val) {
- if (!val) {
- return val;
- }
- var parts = parse_charm_id(val),
- self = this;
- parts.revision = parseInt(parts.revision, 10);
- Y.each(parts, function(value, key) {
- self._set(key, value);
});
- this._set(
- 'charm_store_path', _calculate_charm_store_path(parts));
- this._set('full_name', _calculate_full_charm_name(parts));
- return _reconsititute_charm_id(parts);
+ if (data.owner === 'charmers') {
+ delete data.owner;
+ }
+ return data;
},
- validator: function(val) {
- var parts = parse_charm_id(val);
- return (parts && Y.Lang.isValue(parts.revision));
+ compare: function(other, relevance, otherRelevance) {
+ // Official charms sort before owned charms.
+ // If !X.owner, that means it is owned by charmers.
+ var owner = this.get('owner'),
+ otherOwner = other.get('owner');
+ if (!owner && otherOwner) {
+ return -1;
+ } else if (owner && !otherOwner) {
+ return 1;
+ // Relevance is next most important.
+ } else if (relevance && (relevance !== otherRelevance)) {
+ // Higher relevance comes first.
+ return otherRelevance - relevance;
+ // Otherwise sort by package name, then by owner, then by revision.
+ } else {
+ return (
+ (this.get('package_name').localeCompare(
+ other.get('package_name'))) ||
+ (owner ? owner.localeCompare(otherOwner) : 0) ||
+ (this.get('revision') - other.get('revision')));
+ }
}
- },
- // All of the below are loaded except as noted.
- bzr_branch: {writeOnce: true},
- charm_store_path: {readOnly: true}, // calculated
- config: {writeOnce: true},
- description: {writeOnce: true},
- full_name: {readOnly: true}, // calculated
- is_subordinate: {writeOnce: true},
- last_change:
- { writeOnce: true,
+ }, {
+ ATTRS: {
+ id: {
+ writeOnce: true,
+ validator: function(val) {
+ return Y.Lang.isString(val) && !!charm_id_re.exec(val);
+ }
+ },
+ bzr_branch: {writeOnce: true},
+ charm_store_path: {writeOnce: true},
+ config: {writeOnce: true},
+ description: {writeOnce: true},
+ full_name: {writeOnce: true},
+ is_subordinate: {writeOnce: true},
+ last_change: {
+ writeOnce: true,
setter: function(val) {
// Normalize created value from float to date object.
if (val && val.created) {
@@ -201,102 +174,42 @@
return val;
}
},
- maintainer: {writeOnce: true},
- metadata: {writeOnce: true},
- package_name: {readOnly: true}, // calculated
- owner: {readOnly: true}, // calculated
- peers: {writeOnce: true},
- proof: {writeOnce: true},
- provides: {writeOnce: true},
- requires: {writeOnce: true},
- revision: {readOnly: true}, // calculated
- scheme: {readOnly: true}, // calculated
- series: {readOnly: true}, // calculated
- summary: {writeOnce: true},
- url: {writeOnce: true}
- }
- });
+ maintainer: {writeOnce: true},
+ metadata: {writeOnce: true},
+ package_name: {writeOnce: true},
+ owner: {writeOnce: true},
+ peers: {writeOnce: true},
+ proof: {writeOnce: true},
+ provides: {writeOnce: true},
+ requires: {writeOnce: true},
+ revision: {
+ writeOnce: true,
+ setter: function(val) {
+ if (Y.Lang.isValue(val)) {
+ val = parseInt(val, 10);
+ }
+ return val;
+ }
+ },
+ scheme: {
+ value: 'cs',
+ writeOnce: true,
+ setter: function(val) {
+ if (!Y.Lang.isValue(val)) {
+ val = 'cs';
+ }
+ return val;
+ }
+ },
+ series: {writeOnce: true},
+ summary: {writeOnce: true},
+ url: {writeOnce: true}
+ }
+ });
models.Charm = Charm;
var CharmList = Y.Base.create('charmList', Y.ModelList, [], {
- model: Charm,
-
- initializer: function() {
- this._baseIdHash = {}; // base id (without revision) to array of charms.
- },
-
- _addToBaseIdHash: function(charm) {
- var baseId = charm.get('scheme') + ':' + charm.get('full_name'),
- matches = this._baseIdHash[baseId];
- if (!matches) {
- matches = this._baseIdHash[baseId] = [];
- }
- matches.push(charm);
- // Note that we don't handle changing baseIds or removed charms because
- // that should not happen.
- // Sort on newest charms first.
- matches.sort(function(a, b) {
- var revA = parseInt(a.get('revision'), 10),
- revB = parseInt(b.get('revision'), 10);
- return revB - revA;
- });
- },
-
- add: function() {
- var result = CharmList.superclass.add.apply(this, arguments);
- if (Y.Lang.isArray(result)) {
- Y.each(result, this._addToBaseIdHash, this);
- } else {
- this._addToBaseIdHash(result);
- }
- return result;
- },
-
- getOneByBaseId: function(id) {
- var match = parse_charm_id(id),
- baseId = match && _calculate_base_charm_id(match),
- charms = baseId && this._baseIdHash[baseId];
- return charms && charms[0];
- },
-
- loadOneByBaseId: function(id, options) {
- var match = parse_charm_id(id);
- if (match) {
- if (!options.force) {
- var charm = this.getOneByBaseId(_calculate_base_charm_id(match));
- if (charm) {
- if (options.success) {
- options.success(charm);
- }
- return;
- }
- }
- var path = _calculate_charm_store_path(match),
- self = this;
- options.charm_store.loadByPath(
- path,
- { success: function(data) {
- // We fall back to 0 for revision. Some records do not have one
- // still in the charm store, such as
- // http://jujucharms.com/charms/precise/appflower/json (as of this
- // writing).
- match.revision = data.store_revision || 0;
- id = _reconsititute_charm_id(match);
- charm = self.getById(id);
- if (!charm) {
- charm = self.add({id: id});
- charm.setAttrs(_clean_charm_data(data));
- charm.loaded = true;
- }
- if (options.success) {
- options.success(charm);
- }
- },
- failure: options.failure });
- } else {
- throw id + ' is not a valid base charm id';
- }
- }
+ model: Charm
}, {
ATTRS: {}
});
=== modified file 'app/modules.js'
--- app/modules.js 2012-10-19 01:58:34 +0000
+++ app/modules.js 2012-10-23 20:09:21 +0000
@@ -1,6 +1,6 @@
GlobalConfig = {
// Uncomment for debug versions of YUI.
- //filter: 'debug',
+ filter: 'debug',
// Uncomment for verbose logging of YUI
debug: false,
@@ -83,6 +83,7 @@
},
'juju-charm-models': {
+ requires: ['juju-charm-id'],
fullpath: '/juju-ui/models/charm.js'
},
@@ -103,6 +104,7 @@
},
'juju-charm-store': {
+ requires: ['juju-charm-id'],
fullpath: '/juju-ui/store/charm.js'
},
=== modified file 'app/store/charm.js'
--- app/store/charm.js 2012-10-19 01:53:03 +0000
+++ app/store/charm.js 2012-10-23 20:09:21 +0000
@@ -16,9 +16,11 @@
}
});
},
- // The query can be a string that is passed directly to the search url, or a
- // hash that is marshalled to the correct format (e.g., {series:precise,
- // owner:charmers}).
+ // The query can be a string that is passed directly to the search url, or
+ // a hash that is marshalled to the correct format (e.g.,
+ // {series:precise owner:charmers}). The method returns CharmId instances
+ // grouped by series and ordered within the groups according to the
+ // CharmId compare function.
find: function(query, options) {
if (!Y.Lang.isString(query)) {
var tmp = [];
@@ -38,32 +40,33 @@
console.log('results update', result_set);
options.success(
this._normalizeCharms(
- result_set.results, options.defaultSeries));
+ result_set.results, options.list, options.defaultSeries));
}, this),
'failure': options.failure
}});
},
- // Stash the base id on each charm, convert the official "charmers" owner to
- // an empty owner, and group the charms within series. The series are
- // arranged with first the defaultSeries, if any, and then all other
- // available series arranged from newest to oldest. Within each series,
- // official charms come first, sorted by relevance if available and package
- // name otherwise; and then owned charms follow, sorted again by relevance
- // if available and package name otherwise.
- _normalizeCharms: function(charms, defaultSeries) {
- var hash = {};
- Y.each(charms, function(charm) {
- charm.baseId = charm.series + '/' + charm.name;
- if (charm.owner === 'charmers') {
- charm.owner = null;
- } else {
- charm.baseId = '~' + charm.owner + '/' + charm.baseId;
- }
- charm.baseId = 'cs:' + charm.baseId;
- if (!Y.Lang.isValue(hash[charm.series])) {
- hash[charm.series] = [];
- }
- hash[charm.series].push(charm);
+ // Convert the charm data into Charm instances, using only id and
+ // relevance. Group them into series. The series are arranged with first
+ // the defaultSeries, if any, and then all other available series arranged
+ // from newest to oldest. Within each series, official charms come first,
+ // sorted by relevance if available and package name otherwise; and then
+ // owned charms follow, sorted again by relevance if available and package
+ // name otherwise.
+ _normalizeCharms: function(results, list, defaultSeries) {
+ var hash = {},
+ relevances = {};
+ Y.each(results, function(result) {
+ var charm = list.getById(result.store_url);
+ if (!charm) {
+ charm = list.add(
+ { id: result.store_url, summary: result.summary });
+ }
+ var series = charm.get('series');
+ if (!Y.Lang.isValue(hash[series])) {
+ hash[series] = [];
+ }
+ hash[series].push(charm);
+ relevances[charm.get('id')] = result.relevance;
});
var series_names = Y.Object.keys(hash);
series_names.sort(function(a, b) {
@@ -71,42 +74,19 @@
return -1;
} else if (a !== defaultSeries && b === defaultSeries) {
return 1;
- } else if (a > b) {
- return -1;
- } else if (a < b) {
- return 1;
} else {
- return 0;
+ return -a.localeCompare(b);
}
});
return Y.Array.map(series_names, function(name) {
var charms = hash[name];
charms.sort(function(a, b) {
- // If !a.owner, that means it is owned by charmers.
- if (!a.owner && b.owner) {
- return -1;
- } else if (a.owner && !b.owner) {
- return 1;
- } else if (a.relevance < b.relevance) {
- return 1; // Higher relevance comes first.
- } else if (a.relevance > b.relevance) {
- return -1;
- } else if (a.name < b.name) {
- return -1;
- } else if (a.name > b.name) {
- return 1;
- } else if (a.owner < b.owner) {
- return -1;
- } else if (a.owner > b.owner) {
- return 1;
- } else {
- return 0;
- }
+ return a.compare(
+ b, relevances[a.get('id')], relevances[b.get('id')]);
});
return {series: name, charms: hash[name]};
});
}
-
}, {
ATTRS: {
datasource: {
=== modified file 'app/templates/charm-search-result.handlebars'
--- app/templates/charm-search-result.handlebars 2012-10-19 01:44:45 +0000
+++ app/templates/charm-search-result.handlebars 2012-10-23 20:09:21 +0000
@@ -7,14 +7,11 @@
{{#charms}}
<li class="charm-entry">
<div>
-
<button class="btn btn-primary deploy"
- data-url="{{baseId}}">Deploy</button>
- <a class="charm-detail" href="{{baseId}}">
- {{#if owner}}{{owner}}/{{/if}}{{name}}</a>
-
+ data-url="{{id}}">Deploy</button>
+ <a class="charm-detail" href="{{id}}">
+ {{#if owner}}{{owner}}/{{/if}}{{package_name}}</a>
<div class="charm-summary">{{summary}}</div>
-
</div>
</li>
{{/charms}}
=== modified file 'app/views/charm-search.js'
--- app/views/charm-search.js 2012-10-19 01:53:03 +0000
+++ app/views/charm-search.js 2012-10-23 20:09:21 +0000
@@ -50,7 +50,8 @@
self.set('resultEntries', charms);
},
failure: Y.bind(this._showErrors, this),
- defaultSeries: this.get('defaultSeries')
+ defaultSeries: this.get('defaultSeries'),
+ list: this.get('charms')
});
}
});
@@ -63,7 +64,8 @@
self.set('defaultEntries', charms);
},
failure: Y.bind(this._showErrors, this),
- defaultSeries: this.get('defaultSeries')
+ defaultSeries: this.get('defaultSeries'),
+ list: this.get('charms')
});
}
});
@@ -81,8 +83,17 @@
searchText = this.get('searchText'),
defaultEntries = this.get('defaultEntries'),
resultEntries = this.get('resultEntries'),
- entries = searchText ? resultEntries : defaultEntries;
- container.setHTML(this.template({charms: entries}));
+ raw_entries = searchText ? resultEntries : defaultEntries,
+ entries = raw_entries && raw_entries.map(
+ function(data) {
+ return {
+ series: data.series,
+ charms: data.charms.map(
+ function(charm) { return charm.getAttrs(); })
+ };
+ }
+ );
+ container.setHTML(this.template({ charms: entries }));
return this;
},
showDetails: function(ev) {
@@ -156,7 +167,7 @@
views.CharmDescriptionView = CharmDescriptionView;
var CharmConfigurationView = Y.Base.create(
- 'CharmCollectionView', Y.View, [views.JujuBaseView], {
+ 'CharmConfigurationView', Y.View, [views.JujuBaseView], {
template: views.Templates['charm-pre-configuration'],
tooltip: null,
configFileContent: null,
@@ -399,7 +410,8 @@
charmsSearchPanel = new CharmCollectionView(
{ container: charmsSearchPanelNode,
app: app,
- charmStore: charmStore }),
+ charmStore: charmStore,
+ charms: charms }),
descriptionPanelNode = Y.Node.create(),
descriptionPanel = new CharmDescriptionView(
{ container: descriptionPanelNode,
@@ -432,15 +444,19 @@
contentNode.append(panels[config.name].get('container'));
if (config.charmId) {
newPanel.set('model', null); // Clear out the old.
- charms.loadOneByBaseId(
- config.charmId,
- { success: function(charm) {newPanel.set('model', charm);},
- failure: function(data) {
- console.log('error loading charm', data);
- newPanel.fire('changePanel', {name: 'charms'});
- },
- charm_store: charmStore
- });
+ var charm = charms.getById(config.charmId);
+ if (charm.loaded) {
+ newPanel.set('model', charm);
+ } else {
+ charm.load(charmStore, function(err, response) {
+ if (err) {
+ console.log('error loading charm', response);
+ newPanel.fire('changePanel', {name: 'charms'});
+ } else {
+ newPanel.set('model', charm);
+ }
+ });
+ }
} else { // This is the search panel.
newPanel.render();
}
=== modified file 'test/data/search_results.json'
--- test/data/search_results.json 2012-10-19 01:44:45 +0000
+++ test/data/search_results.json 2012-10-23 20:09:21 +0000
@@ -1,63 +1,70 @@
{
"matches": 7,
- "charm_total": 466,
+ "charm_total": 469,
"results_size": 7,
- "search_time": 0.0011610984802246094,
+ "search_time": 0.001168966293334961,
"results": [
{
"data_url": "/charms/precise/cassandra/json",
"name": "cassandra",
+ "store_url": "cs:precise/cassandra-2",
"series": "precise",
"summary": "distributed storage system for structured data",
- "relevance": 29.552706339212623,
+ "relevance": 29.58930787782692,
"owner": "charmers"
},
{
"data_url": "/charms/oneiric/cassandra/json",
"name": "cassandra",
+ "store_url": "cs:~charmers/oneiric/cassandra-0",
"series": "oneiric",
"summary": "distributed storage system for structured data",
- "relevance": 29.429741180715094,
+ "relevance": 29.46580425464922,
"owner": "charmers"
},
{
"data_url": "/~jjo/precise/cassandra/json",
"name": "cassandra",
+ "store_url": "cs:~jjo/precise/cassandra-12",
"series": "precise",
"summary": "distributed storage system for structured data",
- "relevance": 28.060048153642214,
+ "relevance": 28.090375175505876,
"owner": "jjo"
},
{
"data_url": "/~ev/precise/errors/json",
"name": "errors",
+ "store_url": "cs:~ev/precise/errors-0",
"series": "precise",
"summary": "https://errors.ubuntu.com";,
- "relevance": 13.168754948011127,
+ "relevance": 13.18957931651269,
"owner": "ev"
},
{
"data_url": "/~ev/precise/daisy/json",
"name": "daisy",
+ "store_url": "cs:~ev/precise/daisy-15",
"series": "precise",
"summary": "Daisy error reporting server",
- "relevance": 12.721871518319244,
+ "relevance": 12.767880577035612,
"owner": "ev"
},
{
"data_url": "/~ev/precise/daisy-retracer/json",
"name": "daisy-retracer",
+ "store_url": "cs:~ev/precise/daisy-retracer-8",
"series": "precise",
"summary": "Daisy error reporting server retracer",
- "relevance": 12.662102672474589,
+ "relevance": 12.539732674900428,
"owner": "ev"
},
{
"data_url": "/~negronjl/oneiric/cassandra/json",
"name": "cassandra",
+ "store_url": "cs:~negronjl/oneiric/cassandra-0",
"series": "oneiric",
"summary": "distributed storage system for structured data",
- "relevance": 30.40622159052724,
+ "relevance": 30.451103781408868,
"owner": "negronjl"
}
]
=== modified file 'test/data/series_search_results.json'
--- test/data/series_search_results.json 2012-10-19 01:44:45 +0000
+++ test/data/series_search_results.json 2012-10-23 20:09:21 +0000
@@ -1,12 +1,13 @@
{
"matches": 5,
- "charm_total": 466,
+ "charm_total": 469,
"results_size": 5,
- "search_time": 0.0008029937744140625,
+ "search_time": 0.0007879734039306641,
"results": [
{
"data_url": "/charms/quantal/glance/json",
"name": "glance",
+ "store_url": "cs:~charmers/quantal/glance-2",
"series": "quantal",
"summary": "OpenStack Image Registry and Delivery Service",
"relevance": 0.0,
@@ -15,6 +16,7 @@
{
"data_url": "/charms/quantal/nova-cloud-controller/json",
"name": "nova-cloud-controller",
+ "store_url": "cs:~charmers/quantal/nova-cloud-controller-1",
"series": "quantal",
"summary": "Openstack nova controller node.",
"relevance": 0.0,
@@ -23,6 +25,7 @@
{
"data_url": "/charms/quantal/nova-volume/json",
"name": "nova-volume",
+ "store_url": "cs:~charmers/quantal/nova-volume-0",
"series": "quantal",
"summary": "OpenStack Compute - storage",
"relevance": 0.0,
@@ -31,6 +34,7 @@
{
"data_url": "/charms/quantal/nova-compute/json",
"name": "nova-compute",
+ "store_url": "cs:~charmers/quantal/nova-compute-1",
"series": "quantal",
"summary": "OpenStack compute",
"relevance": 0.0,
@@ -39,6 +43,7 @@
{
"data_url": "/charms/quantal/nyancat/json",
"name": "nyancat",
+ "store_url": "cs:~charmers/quantal/nyancat-0",
"series": "quantal",
"summary": "Nyancat telnet server",
"relevance": 0.0,
=== modified file 'test/test_app.js'
--- test/test_app.js 2012-10-19 01:53:03 +0000
+++ test/test_app.js 2012-10-23 20:09:21 +0000
@@ -88,7 +88,7 @@
// charms also require a mapping but only a name, not a function
app.getModelURL(wp_charm).should.equal(
- '/charms/charms/precise/wordpress/json');
+ '/charms/charms/precise/wordpress-6/json');
});
it('should display the configured environment name', function() {
=== modified file 'test/test_charm_configuration.js'
--- test/test_charm_configuration.js 2012-10-20 18:17:23 +0000
+++ test/test_charm_configuration.js 2012-10-23 20:09:21 +0000
@@ -91,7 +91,7 @@
received_charm_url = charm_url;
received_service_name = service_name;
}},
- charm = new models.Charm({id: 'precise/mysql-7'}),
+ charm = new models.Charm({id: 'cs:precise/mysql-7'}),
view = new views.CharmConfigurationView(
{ container: container,
model: charm,
@@ -119,7 +119,7 @@
received_config = config;
received_num_units = num_units;
}},
- charm = new models.Charm({id: 'precise/mysql-7'}),
+ charm = new models.Charm({id: 'cs:precise/mysql-7'}),
view = new views.CharmConfigurationView(
{ container: container,
model: charm,
=== modified file 'test/test_charm_search.js'
--- test/test_charm_search.js 2012-10-19 01:53:03 +0000
+++ test/test_charm_search.js 2012-10-23 20:09:21 +0000
@@ -5,7 +5,7 @@
searchResult = '{"results": [{"data_url": "this is my URL", ' +
'"name": "membase", "series": "precise", "summary": ' +
'"Membase Server", "relevance": 8.728194117350437, ' +
- '"owner": "charmers"}]}';
+ '"owner": "charmers", "store_url": "cs:precise/membase-6"}]}';
before(function(done) {
Y = YUI(GlobalConfig).use(
@@ -85,7 +85,7 @@
searchTriggered.should.equal(true);
node.one('.charm-entry .btn.deploy').getData('url').should.equal(
- 'cs:precise/membase');
+ 'cs:precise/membase-6');
});
it('must be able to trigger charm details', function() {
@@ -107,7 +107,7 @@
testing: true
}),
node = panel.node;
- db.charms.add({id: 'cs:precise/membase'});
+ db.charms.add({id: 'cs:precise/membase-6'});
panel.show();
var field = Y.one('#charm-search-field');
@@ -138,7 +138,7 @@
testing: true
}),
node = panel.node,
- charm = db.charms.add({id: 'cs:precise/membase'});
+ charm = db.charms.add({id: 'cs:precise/membase-6'});
charm.loaded = true;
panel.show();
var field = Y.one('#charm-search-field');
=== modified file 'test/test_charm_store.js'
--- test/test_charm_store.js 2012-10-19 01:53:03 +0000
+++ test/test_charm_store.js 2012-10-23 20:09:21 +0000
@@ -8,9 +8,10 @@
before(function(done) {
Y = YUI(GlobalConfig).use(
'datasource-local', 'json-stringify', 'juju-charm-store',
- 'datasource-io', 'io', 'array-extras',
+ 'datasource-io', 'io', 'array-extras', 'juju-charm-models',
function(Y) {
juju = Y.namespace('juju');
+ models = Y.namespace('juju.models');
done();
});
});
@@ -108,19 +109,20 @@
results.length.should.equal(2);
results[0].series.should.equal('precise');
Y.Array.map(results[0].charms, function(charm) {
- return charm.owner;
- }).should.eql([null, 'jjo', 'ev', 'ev', 'ev']);
+ return charm.get('owner');
+ }).should.eql([undefined, 'jjo', 'ev', 'ev', 'ev']);
Y.Array.map(results[0].charms, function(charm) {
- return charm.baseId;
+ return charm.get('id');
}).should.eql([
- 'cs:precise/cassandra',
- 'cs:~jjo/precise/cassandra',
- 'cs:~ev/precise/errors',
- 'cs:~ev/precise/daisy',
- 'cs:~ev/precise/daisy-retracer']);
+ 'cs:precise/cassandra-2',
+ 'cs:~jjo/precise/cassandra-12',
+ 'cs:~ev/precise/errors-0',
+ 'cs:~ev/precise/daisy-15',
+ 'cs:~ev/precise/daisy-retracer-8']);
done();
},
- failure: assert.fail
+ failure: assert.fail,
+ list: new models.CharmList()
});
});
@@ -135,7 +137,8 @@
results[0].series.should.equal('oneiric');
done();
},
- failure: assert.fail
+ failure: assert.fail,
+ list: new models.CharmList()
});
});
@@ -149,7 +152,7 @@
results.length.should.equal(1);
results[0].series.should.equal('quantal');
Y.Array.map(results[0].charms, function(charm) {
- return charm.name;
+ return charm.get('package_name');
}).should.eql([
'glance',
'nova-cloud-controller',
@@ -158,9 +161,9 @@
'nyancat']);
done();
},
- failure: assert.fail
+ failure: assert.fail,
+ list: new models.CharmList()
});
});
-
});
})();
=== modified file 'test/test_model.js'
--- test/test_model.js 2012-10-19 01:53:03 +0000
+++ test/test_model.js 2012-10-23 20:09:21 +0000
@@ -19,56 +19,19 @@
var _ = expect(charm.get('owner')).to.not.exist;
charm.get('full_name').should.equal('precise/openstack-dashboard');
charm.get('charm_store_path').should.equal(
- 'charms/precise/openstack-dashboard/json');
+ 'charms/precise/openstack-dashboard-0/json');
});
it('must convert timestamps into time objects', function() {
var time = 1349797266.032,
date = new Date(time),
charm = new models.Charm(
- { id: 'precise/foo', last_change: {created: time / 1000} });
+ { id: 'cs:precise/foo-9', last_change: {created: time / 1000} });
charm.get('last_change').created.should.eql(date);
});
});
- describe('charm id helper functions', function() {
- var Y, models;
-
- before(function(done) {
- Y = YUI(GlobalConfig).use('juju-models', function(Y) {
- models = Y.namespace('juju.models');
- done();
- });
- });
-
- it('must parse fully qualified names', function() {
- // undefined never equals undefined.
- var res = models.parse_charm_id('cs:precise/openstack-dashboard-0');
- res.scheme.should.equal('cs');
- var _ = expect(res.owner).to.not.exist;
- res.series.should.equal('precise');
- res.package_name.should.equal('openstack-dashboard');
- res.revision.should.equal('0');
- });
-
- it('must parse names without revisions', function() {
- var res = models.parse_charm_id('cs:precise/openstack-dashboard'),
- _ = expect(res.revision).to.not.exist;
- });
-
- it('must parse fully qualified names with owners', function() {
- models.parse_charm_id('cs:~bac/precise/openstack-dashboard-0').owner
- .should.equal('bac');
- });
-
- it('must parse fully qualified names with hyphenated owners', function() {
- models.parse_charm_id('cs:~alt-bac/precise/openstack-dashboard-0').owner
- .should.equal('alt-bac');
- });
-
- });
-
describe('juju models', function() {
var Y, models;
@@ -81,15 +44,16 @@
it('must be able to create charm', function() {
var charm = new models.Charm(
- {id: 'cs:~bac/precise/openstack-dashboard-0'});
+ {id: 'cs:~alt-bac/precise/openstack-dashboard-0'});
charm.get('scheme').should.equal('cs');
- charm.get('owner').should.equal('bac');
+ charm.get('owner').should.equal('alt-bac');
charm.get('series').should.equal('precise');
charm.get('package_name').should.equal('openstack-dashboard');
charm.get('revision').should.equal(0);
- charm.get('full_name').should.equal('~bac/precise/openstack-dashboard');
+ charm.get('full_name').should.equal(
+ '~alt-bac/precise/openstack-dashboard');
charm.get('charm_store_path').should.equal(
- '~bac/precise/openstack-dashboard/json');
+ '~alt-bac/precise/openstack-dashboard-0/json');
});
it('must be able to parse real-world charm names', function() {
@@ -97,7 +61,12 @@
charm.get('full_name').should.equal('precise/openstack-dashboard');
charm.get('package_name').should.equal('openstack-dashboard');
charm.get('charm_store_path').should.equal(
- 'charms/precise/openstack-dashboard/json');
+ 'charms/precise/openstack-dashboard-0/json');
+ charm.get('scheme').should.equal('cs');
+ var _ = expect(charm.get('owner')).to.not.exist;
+ charm.get('series').should.equal('precise');
+ charm.get('package_name').should.equal('openstack-dashboard');
+ charm.get('revision').should.equal(0);
});
it('must be able to parse individually owned charms', function() {
@@ -108,14 +77,28 @@
charm.get('full_name').should.equal('~marco-ceppi/precise/wordpress');
charm.get('package_name').should.equal('wordpress');
charm.get('charm_store_path').should.equal(
- '~marco-ceppi/precise/wordpress/json');
+ '~marco-ceppi/precise/wordpress-17/json');
+ charm.get('revision').should.equal(17);
});
it('must reject bad charm ids.', function() {
- var charm = new models.Charm({id: 'foobar'});
- var _ = expect(charm.get('id')).to.not.exist;
- charm.set('id', 'barfoo');
- _ = expect(charm.get('id')).to.not.exist;
+ try {
+ var charm = new models.Charm({id: 'foobar'});
+ assert.fail('Should have thrown an error');
+ } catch (e) {
+ e.should.equal(
+ 'Developers must initialize charms with a well-formed id.');
+ }
+ });
+
+ it('must reject missing charm ids at initialization.', function() {
+ try {
+ var charm = new models.Charm();
+ assert.fail('Should have thrown an error');
+ } catch (e) {
+ e.should.equal(
+ 'Developers must initialize charms with a well-formed id.');
+ }
});
it('must be able to create charm list', function() {
@@ -403,7 +386,7 @@
});
it('will throw an exception with non-read sync', function() {
- var charm = new models.Charm({id: 'local:precise/foo'});
+ var charm = new models.Charm({id: 'local:precise/foo-4'});
try {
charm.sync('create');
assert.fail('Should have thrown an error');
@@ -424,39 +407,40 @@
}
});
- it('throws an error if you do not pass a get_charm function', function() {
- var charm = new models.Charm({id: 'local:precise/foo'});
- try {
- charm.sync('read', {});
- assert.fail('Should have thrown an error');
- } catch (e) {
- e.should.equal(
- 'You must supply a get_charm function.');
- }
- try {
- charm.sync('read', {env: 42});
- assert.fail('Should have thrown an error');
- } catch (e) {
- e.should.equal(
- 'You must supply a get_charm function.');
- }
- try {
- charm.sync('read', {charm_store: 42});
- assert.fail('Should have thrown an error');
- } catch (e) {
- e.should.equal(
- 'You must supply a get_charm function.');
- }
- });
+ it('throws an error if you do not pass get_charm or loadByPath function',
+ function() {
+ var charm = new models.Charm({id: 'local:precise/foo-4'});
+ try {
+ charm.sync('read', {});
+ assert.fail('Should have thrown an error');
+ } catch (e) {
+ e.should.equal(
+ 'You must supply a get_charm or loadByPath function.');
+ }
+ try {
+ charm.sync('read', {env: 42});
+ assert.fail('Should have thrown an error');
+ } catch (e) {
+ e.should.equal(
+ 'You must supply a get_charm or loadByPath function.');
+ }
+ try {
+ charm.sync('read', {charm_store: 42});
+ assert.fail('Should have thrown an error');
+ } catch (e) {
+ e.should.equal(
+ 'You must supply a get_charm or loadByPath function.');
+ }
+ });
it('must send request to juju environment for local charms', function() {
- var charm = new models.Charm({id: 'local:precise/foo'}).load(env);
+ var charm = new models.Charm({id: 'local:precise/foo-4'}).load(env);
assert(!charm.loaded);
conn.last_message().op.should.equal('get_charm');
});
it('must handle success from local charm request', function(done) {
- var charm = new models.Charm({id: 'local:precise/foo'}).load(
+ var charm = new models.Charm({id: 'local:precise/foo-4'}).load(
env,
function(err, response) {
assert(!err);
@@ -471,7 +455,7 @@
});
it('must handle failure from local charm request', function(done) {
- var charm = new models.Charm({id: 'local:precise/foo'}).load(
+ var charm = new models.Charm({id: 'local:precise/foo-4'}).load(
env,
function(err, response) {
assert(err);
@@ -490,22 +474,18 @@
{ responseText: Y.JSON.stringify(
{ summary: 'wowza', subordinate: true, store_revision: 7 })});
- var list = new models.CharmList();
- list.loadOneByBaseId(
- 'cs:precise/foo',
- { success: function(charm) {
+ var charm = new models.Charm({id: 'cs:precise/foo-7'});
+ charm.load(
+ charm_store,
+ function(err, data) {
+ if (err) { assert.fail('should succeed!'); }
assert(charm.loaded);
charm.get('summary').should.equal('wowza');
charm.get('is_subordinate').should.equal(true);
charm.get('scheme').should.equal('cs');
charm.get('revision').should.equal(7);
charm.get('id').should.equal('cs:precise/foo-7');
- list.getById('cs:precise/foo-7').should.equal(charm);
- list.getOneByBaseId('cs:precise/foo').should.equal(charm);
done();
- },
- failure: assert.fail,
- charm_store: charm_store
});
});
@@ -521,15 +501,14 @@
original.apply(datasource, [e]);
};
data.push({responseText: Y.JSON.stringify({darn_it: 'uh oh!'})});
- list.loadOneByBaseId(
- 'cs:precise/foo',
- { success: assert.fail,
- failure: function(err) {
- var _ = expect(list.getOneByBaseId('cs:precise/foo'))
- .to.not.exist;
- done();
- },
- charm_store: charm_store
+ var charm = new models.Charm({id: 'cs:precise/foo-7'});
+ charm.load(
+ charm_store,
+ function(err, data) {
+ if (!err) {
+ assert.fail('should fail!');
+ }
+ done();
});
});
=== modified file 'test/test_service_view.js'
--- test/test_service_view.js 2012-10-12 18:27:36 +0000
+++ test/test_service_view.js 2012-10-23 20:09:21 +0000
@@ -30,7 +30,7 @@
app = { env: env, db: db,
getModelURL: function(model, intent) {
return model.get('name'); }};
- charm = new models.Charm({id: 'cs:precise/mysql',
+ charm = new models.Charm({id: 'cs:precise/mysql-5',
description: 'A DB'});
db.charms.add([charm]);
// Add units sorted by id as that is what we expect from the server.
@@ -40,7 +40,7 @@
]);
service = new models.Service({
id: 'mysql',
- charm: 'cs:precise/mysql',
+ charm: 'cs:precise/mysql-5',
unit_count: db.units.size(),
exposed: false});
Follow ups
