Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

fix issue #835 more careful about caching results of expandInstanceFo…

…rEnv()
  • Loading branch information...
commit 54dcebe26ae65813126588b0c8b86dbe43ca8324 1 parent b254d13
@drewfish drewfish authored
View
2  lib/app/addons/rs/url.js
@@ -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';
View
279 lib/app/autoload/store.server.js
@@ -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
/**
@@ -529,7 +527,6 @@ 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.
@@ -537,61 +534,49 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
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);
},
@@ -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
@@ -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);
},
@@ -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,
@@ -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
);
},
View
31 tests/unit/lib/app/autoload/test-store.server.js
@@ -261,21 +261,28 @@ YUI().use(
});
},
- 'expandInstance caching': function() {
- var inInstance = {
+ 'expandSpec caching': function() {
+ var inSpec = {
base: 'a',
type: 'c'
};
- var context = {};
- var key = Y.JSON.stringify([inInstance, ['*'], context.lang]);
- store._expandInstanceCache.server[key] = { x: 'y' };
- store.expandInstance(inInstance, context, function(err, outInstance) {
- A.isNull(err);
- A.areEqual(4, Object.keys(outInstance).length);
- A.areEqual('a', outInstance.base);
- A.areEqual('c', outInstance.type);
- A.areEqual('y', outInstance.x);
- });
+ var env = 'server';
+ var ctx = {};
+ var key = Y.JSON.stringify([env, ctx, inSpec]);
+ store._expandSpecCache[key] = { x: 'y' };
+ var outSpec = store._expandSpec(env, ctx, inSpec);
+ A.isObject(outSpec);
+ A.areEqual(1, Object.keys(outSpec).length);
+ A.areEqual('y', outSpec.x);
+ },
+
+ 'getMojitTypeDetails caching': function() {
+ var key = Y.JSON.stringify(['server', ['*'], 'en', 'x']);
+ store._getMojitTypeDetailsCache[key] = { x: 'y' };
+ var details = store.getMojitTypeDetails('server', {lang: 'en'}, 'x');
+ A.isObject(details);
+ A.areEqual(1, Object.keys(details).length);
+ A.areEqual('y', details.x);
},
'multi preload': function() {
Please sign in to comment.
Something went wrong with that request. Please try again.