Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

fix issue #835 more careful about caching results of expandInstanceForEnv #845

Merged
merged 2 commits into from Dec 7, 2012
Merged

fix issue #835 more careful about caching results of expandInstanceForEnv #845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54dcebe
fix issue #835 more careful about caching results of expandInstanceFo…
drewfish Dec 6, 2012
a82128c
added test for problem found in issue #843
drewfish Dec 7, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/app/addons/rs/url.js
View file
Open in desktop
Expand Up @@ -161,7 +161,7 @@ YUI.add('addon-rs-url', function(Y, NAME) {
var ress = this.get('host').getResources(evt.args.env, evt.args.ctx, {type: 'mojit', name: evt.args.mojitType});

if (!ress || !ress[0]) {
throw new Error('Unable to compute assets root for mojit "' + evt.mojit.type + '". This usually happens when trying to run a mojit that does not exist.');
throw new Error('Unable to compute assets root for mojit "' + evt.args.mojitType + '". This usually happens when trying to run a mojit that does not exist.');
}

evt.mojit.assetsRoot = ress[0].url + '/assets';
Expand Down
279 changes: 148 additions & 131 deletions lib/app/autoload/store.server.js
View file
Open in desktop
Expand Up @@ -179,15 +179,13 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
this._routesCache = {}; // serialized context: route
this._appConfigCache = {}; //cache for the app config
this._validateContextCache = {}; // ctx: error string or "VALID"
this._getMojitTypeDetailsCache = {}; // env+posl+lang+mojitType: value
this._expandSpecCache = {}; // env+ctx+spec: value

this._appRVs = []; // array of resource versions
this._mojitRVs = {}; // mojitType: array of resource versions
this._appResources = {}; // env: posl: array of resources
this._mojitResources = {}; // env: posl: mojitType: array of resources
this._expandInstanceCache = { // env: cacheKey: instance
client: {},
server: {}
};
this._appPkg = null; // metadata about the applicaions's NPM package

/**
Expand Down Expand Up @@ -529,69 +527,56 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
* @param {function(err,instance)} cb callback used to return the results (or error)
*/
expandInstanceForEnv: function(env, instance, ctx, cb) {

// TODO: fakeInstance could be even more optimized, where
// type has priority over base, and only one of them is really
// needed.
var fakeInstance = {
base: instance.base,
type: instance.type
},
posl = this.selector.getPOSLFromContext(ctx),
// We need to include the lang, since it's a part of the context
// that greatly affects each mojit, yet is not necessarily
// captured in the POSL.
cacheKey = JSON.stringify([fakeInstance, posl, ctx.lang]),
cacheValue = this._expandInstanceCache[env][cacheKey],
spec,
typeDetails,
config,
perf,
newInst;

if (cacheValue) {
newInst = Y.mojito.util.blend(cacheValue, instance);

// Always create a new instance ID, otherwise the old instance
// ID bleeds out of the cache.
newInst.instanceId = Y.guid();
//DEBUGGING: newInst.instanceId += '-instance-server(cache)-' + [newInst.base||'', newInst.type||''].join('-');

cb(null, newInst);
return;
}
newInst,
perf;

// TODO: should this be done here, or somewhere else?
ctx.runtime = env;

if (!instance.instanceId) {
instance.instanceId = Y.guid();
//DEBUGGING: instance.instanceId += '-instance-server-' + [instance.base||'', instance.type||''].join('-');
}
// otherwise this'll show up in the returned instance
instance.base = null;

// spec
try {
spec = Y.mojito.util.copy(this._expandSpec(env, ctx, fakeInstance));
spec = this._expandSpec(env, ctx, fakeInstance);
} catch (err) {
return cb(err);
}
spec.config = spec.config || {};
if (!spec.instanceId) {
spec.instanceId = Y.guid();
//DEBUGGING: spec.instanceId += '-instance-server-' + [spec.base||'', spec.type||''].join('-');
if (!spec.config) {
spec.config = {};
}

// type details
try {
perf = Y.mojito.perf.timeline('mojito', 'store:getMojitTypeDetails',
'expand mojit spec', spec.type);

this.getMojitTypeDetails(env, ctx, spec.type, spec);
typeDetails = this.getMojitTypeDetails(env, ctx, spec.type);
} catch (err2) {
return cb(err2);
} finally {
perf.done();
}
if (spec.defaults && spec.defaults.config) {
config = Y.mojito.util.blend(spec.defaults.config, spec.config);
spec.config = config;
}

this._expandInstanceCache[env][cacheKey] = spec;
cb(null, Y.mojito.util.blend(spec, instance));
// priority (most important to least):
// * instance
// * spec
// * typeDetails
newInst = Y.mojito.util.blend(typeDetails, spec);
newInst = Y.mojito.util.mergeRecursive(newInst, instance);
cb(null, newInst);
},


Expand All @@ -605,8 +590,8 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
* @param {string} env the runtime environment (either `client` or `server`)
* @param {object} ctx the context
* @param {string} mojitType mojit type
* @param {object} dest object in which to place the results
* @return {object} returns the "dest" parameter, which has had details added to it
* @param {object} DEPRECATED: dest object in which to place the results
* @return {object} details about the mojit type
*/
/**
* Fired at the end of the `getMojitTypeDetails()` method to allow
Expand All @@ -621,122 +606,123 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
*/
getMojitTypeDetails: function(env, ctx, mojitType, dest) {
//Y.log('getMojitTypeDetails('+env+', '+JSON.stringify(ctx)+', '+mojitType+')', 'debug', NAME);
var ress,
var posl = this.selector.getPOSLFromContext(ctx),
// We need to include the lang, since it's a part of the context
// that greatly affects each mojit, yet is not necessarily
// captured in the POSL.
cacheKey = JSON.stringify([env, posl, ctx.lang, mojitType]),
cacheValue = this._getMojitTypeDetailsCache[cacheKey],
details,
ress,
r,
res,
engine,
engines = {}, // view engines
posl = this.selector.getPOSLFromContext(ctx),
ctxKey,
module;
res;

if ('shared' === mojitType) {
throw new Error('Mojit name "shared" is special and isn\'t a real mojit.');
}

if (!dest) {
dest = {};
}

if (!dest.assets) {
dest.assets = {};
}
if (!dest.binders) {
dest.binders = {};
}
if (!dest.langs) {
dest.langs = {};
}
if (!dest.models) {
dest.models = {};
}
if (!dest.views) {
dest.views = {};
}
if (!cacheValue) {
details = {};
details.defaults = {};
details.definition = {};
details.assets = {};
details.binders = {};
details.langs = {};
details.models = {};
details.views = {};

dest.definition = {};
dest.defaults = {};
ress = this.getResources(env, ctx, { mojit: mojitType });
for (r = 0; r < ress.length; r += 1) {
res = ress[r];

ress = this.getResources(env, ctx, { mojit: mojitType });
for (r = 0; r < ress.length; r += 1) {
res = ress[r];
if (res.type === 'config') {
if ('definition' === res.source.fs.basename) {
details.definition = this.config.readConfigYCB(res.source.fs.fullPath, ctx);
}
if ('defaults' === res.source.fs.basename) {
details.defaults = this.config.readConfigYCB(res.source.fs.fullPath, ctx);
}
}

if (res.type === 'config') {
if ('definition' === res.source.fs.basename) {
dest.definition = this.config.readConfigYCB(res.source.fs.fullPath, ctx);
if (res.type === 'asset') {
if (env === 'client') {
details.assets[res.name + res.source.fs.ext] = res.url;
} else {
details.assets[res.name + res.source.fs.ext] = res.source.fs.fullPath;
}
}
if ('defaults' === res.source.fs.basename) {
dest.defaults = this.config.readConfigYCB(res.source.fs.fullPath, ctx);

if (res.type === 'controller') {
details.controller = res.yui.name;
}
}

if (res.type === 'asset') {
if (env === 'client') {
dest.assets[res.name + res.source.fs.ext] = res.url;
} else {
dest.assets[res.name + res.source.fs.ext] = res.source.fs.fullPath;
if (res.type === 'yui-lang') {
details.langs[res.yui.lang] = true;
if (res.yui.isRootLang) {
details.defaultLang = res.yui.lang;
}
}
}

if (res.type === 'controller') {
dest.controller = res.yui.name;
}
if (res.type === 'model') {
details.models[res.name] = res.yui.name;
}

if (res.type === 'yui-lang') {
dest.langs[res.yui.lang] = true;
if (res.yui.isRootLang) {
dest.defaultLang = res.yui.lang;
if (res.type === 'binder') {
details.binders[res.name] = res.yui.name;
}
}

if (res.type === 'model') {
dest.models[res.name] = res.yui.name;
if (res.type === 'view') {
if (!details.views[res.name]) {
details.views[res.name] = {};
}
if (env === 'client') {
details.views[res.name]['content-path'] = res.url;
} else {
details.views[res.name]['content-path'] = res.source.fs.fullPath;
}
details.views[res.name].assets = res.view.assets;
details.views[res.name].engine = res.view.engine;
}
}

if (res.type === 'binder') {
dest.binders[res.name] = res.yui.name;
}
// since the binders are not part of the server runtime, but are needed
// to define the binders map, we need synthetically build this.
if (env !== 'client') {
ress = this.getResources('client', ctx, { mojit: mojitType, type: 'binder' });
for (r = 0; r < ress.length; r += 1) {
res = ress[r];

if (res.type === 'view') {
if (!dest.views[res.name]) {
dest.views[res.name] = {};
}
if (env === 'client') {
dest.views[res.name]['content-path'] = res.url;
} else {
dest.views[res.name]['content-path'] = res.source.fs.fullPath;
if (res.type === 'binder') {
details.binders[res.name] = res.yui.name;
}
}
dest.views[res.name].assets = res.view.assets;
dest.views[res.name].engine = res.view.engine;
engines[res.view.engine] = true;
}
}

// since the binders are not part of the server runtime, but are needed
// to define the binders map, we need synthetically build this.
if (env !== 'client') {
ress = this.getResources('client', ctx, { mojit: mojitType, type: 'binder' });
for (r = 0; r < ress.length; r += 1) {
res = ress[r];
// YUI AOP doesn't give plugins enough control, so use
// onHostMethod() and afterHostMethod().
this.fire('getMojitTypeDetails', {
args: {
env: env,
ctx: ctx,
posl: posl,
mojitType: mojitType
},
mojit: details
});

if (res.type === 'binder') {
dest.binders[res.name] = res.yui.name;
}
if (details.defaults && details.defaults.config) {
details.config = Y.mojito.util.blend(details.defaults.config, details.config);
}

cacheValue = details;
this._getMojitTypeDetailsCache[cacheKey] = cacheValue;
}

// YUI AOP doesn't give plugins enough control, so use
// onHostMethod() and afterHostMethod().
this.fire('getMojitTypeDetails', {
args: {
env: env,
ctx: ctx,
posl: posl,
mojitType: mojitType
},
mojit: dest
});
return dest;
if (dest) {
Y.log('The "dest" parameter to store.getMojitTypeDetails() is deprecated.', 'warn', NAME);
Y.mojito.util.mergeRecursive(dest, cacheValue);
}
return Y.mojito.util.copy(cacheValue);
},


Expand Down Expand Up @@ -1577,6 +1563,37 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
*/
// FUTURE: expose this to RS addons?
_expandSpec: function(env, ctx, spec) {
// [issue 835] The context is part of the cache key
// since the spec can vary in the application json.
var cacheKey = JSON.stringify([env, ctx, spec]),
cacheValue = this._expandSpecCache[cacheKey];

// We do the caching in this wrapper because the expandSpec
// algorithm is recursive and we don't want to keep doing
// the cache checks at each iteration.
// We -could- do caching within _expandSpecRecursive() for
// better cache re-use of expanded bases, but it's not
// (currently) expected that apps will have lots (1000s)
// of specs.
if (!cacheValue) {
cacheValue = this._expandSpecRecursive(env, ctx, spec);
this._expandSpecCache[cacheKey] = cacheValue;
}

return Y.mojito.util.copy(cacheValue);
},


/**
* Recursively build the spec by following base.
* @private
* @method _expandSpecRecursive
* @param {string} env the runtime environment (either `client` or `server`)
* @param {object} ctx runtime context
* @param {object} spec spec to expand
* @return {object} expanded sped
*/
_expandSpecRecursive: function(env, ctx, spec) {
var appConfig,
base,
specParts,
Expand Down Expand Up @@ -1611,7 +1628,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) {

spec.base = undefined;
return Y.mojito.util.mergeRecursive(
this._expandSpec(env, ctx, base),
this._expandSpecRecursive(env, ctx, base),
spec
);
},
Expand Down