diff --git a/lib/app/addons/rs/url.js b/lib/app/addons/rs/url.js index 0baa213dd..a9b946f36 100644 --- a/lib/app/addons/rs/url.js +++ b/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'; diff --git a/lib/app/autoload/store.server.js b/lib/app/autoload/store.server.js index 1a043c924..20a6f71eb 100644 --- a/lib/app/autoload/store.server.js +++ b/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 ); }, diff --git a/tests/unit/lib/app/autoload/test-store.server.js b/tests/unit/lib/app/autoload/test-store.server.js index 1269313d1..0cdd27040 100644 --- a/tests/unit/lib/app/autoload/test-store.server.js +++ b/tests/unit/lib/app/autoload/test-store.server.js @@ -261,20 +261,37 @@ 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); + }, + + 'expandInstanceForEnv preserves instanceId': function() { + var inInstance = { + type: 'test_mojit_1', + instanceId: 'foo' + }; + store.expandInstanceForEnv('server', inInstance, {}, function(err, outInstance) { + A.areSame('foo', outInstance.instanceId); }); },