Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

re-enable yaml support #907

Closed
wants to merge 4 commits into from

2 participants

Drew Folta Caridy Patiño
Drew Folta
Owner

closes #804

Caridy Patiño caridy commented on the diff
lib/app/autoload/store.server.js
@@ -364,6 +364,20 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
return appConfig;
},
+
+
+ /**
+ * Does initial preload of many parts of the application and framework.
+ * The full preload is done by preload().
+ *
+ * @method preloadInitial
+ * @return {nothing}
+ */
+ preloadInitial: function() {
Caridy Patiño Owner
caridy added a note

What happen if preloadInitial and then preload is called? Will be good if we support that use-case, so we can do some stuff before getting to the .preload() execution.

Drew Folta Owner

Yeah, I had thought of that too. I will implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Caridy Patiño caridy commented on the diff
lib/app/autoload/store.server.js
@@ -373,11 +387,14 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
*/
preload: function() {
// We need to do an initial sweep to find the resource store addons.
- this.preloadResourceVersions();
+ this.preloadInitial();
Caridy Patiño Owner
caridy added a note

related to the comment above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Caridy Patiño caridy commented on the diff
lib/app/commands/start.js
((20 lines not shown))
}
- try { // Read the package.json config
- pack = JSON.parse(fs.readFileSync(path.join(root, 'package.json'),
- 'utf8'));
- } catch (err2) {
- pack = {};
- }
+ store = Store.createStore({
+ root: root,
+ preload: 'initial',
+ context: inputOptions.context || {}
+ });
+ appConfig = store.getAppConfig();
+
+ pack = store.config.readConfigJSON(path.join(root, 'package.json'));
options.port = params[0] || appConfig.appPort || process.env.PORT || 8666;
Caridy Patiño Owner
caridy added a note

Finally, different port per different context :) nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Caridy Patiño caridy commented on the diff
lib/store.js
@@ -79,7 +83,14 @@ Store.createStore = function(options) {
appConfig: options.appConfig
});
- store.preload();
+ switch (options.preload) {
Caridy Patiño Owner
caridy added a note

isn't switch one of the bad parts in javascript? A couple of "if" statements should be just enough considering that the value was already validated/preset.

Caridy Patiño Owner
caridy added a note

Travis failure:

458+ /home/travis/builds/yahoo/mojito/bin/mojito jslint -p
459store.js
460   1 87,9: Expected 'case' at column 5, not column 9.
461     case 'skip': break;
462   2 87,22: Expected 'break' at column 9, not column 22.
463     case 'skip': break;
464   3 88,9: Expected 'case' at column 5, not column 9.
465     case 'initial':
466   4 89,13: Expected 'store' at column 9, not column 13.
467     store.preloadInitial();
468   5 90,13: Expected 'break' at column 9, not column 13.
469     break;
470   6 91,9: Expected 'default' at column 5, not column 9.
471     default:
472   7 92,13: Expected 'store' at column 9, not column 13.
473     store.preload();
474app/commands/start.js
475   1 76,1: Unexpected '(space)'.
Drew Folta Owner

I had no idea switch was contraindicated. The errors are because I forgot to run jslint, which I'll do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Caridy Patiño
Owner

Closing this in favor of issue #916 which addressed the comments here and more to accomodate this commit for the multi-part application config files as well.

Caridy Patiño caridy closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
68 lib/app/addons/rs/config.js
View
@@ -48,6 +48,7 @@ YUI.add('addon-rs-config', function(Y, NAME) {
this.beforeHostMethod('parseResourceVersion', this.parseResourceVersion, this);
this._jsonCache = {}; // fullPath: contents as JSON object
+ this._simpleCache = {}; // fullPath: contents as JSON object
this._ycbCache = {}; // fullPath: context: YCB config object
this._ycbDims = this._readYcbDimensions();
},
@@ -64,12 +65,14 @@ YUI.add('addon-rs-config', function(Y, NAME) {
/**
- * Same as readConfigSync except the result is cached for future calls.
+ * Reads a JSON file. In mojito, this should generally only be used for
+ * package.json files, and all other mojito config files should instead
+ * be read using readConfigSimple() or readConfigYCB().
* @method readConfigSimple
- * @param {string} fullPath path to JSON or YAML file
+ * @param {string} fullPath path to JSON file
* @return {user-defined} contents of file as an object
*/
- readConfigSimple: function(fullPath) {
+ readConfigJSON: function(fullPath) {
var json,
contents;
if (!existsSync(fullPath)) {
@@ -88,47 +91,56 @@ YUI.add('addon-rs-config', function(Y, NAME) {
return Y.mojito.util.copy(json);
},
+
/**
* Reads and parses a JSON or YAML structured file.
- * @method readConfigSync
+ * @method readConfigSimple
* @param {string} fullPath path to JSON or YAML file
* @return {user-defined} contents of file as an object
*/
- readConfigSync: function (filePath) {
-
+ readConfigSimple: function(fullPath) {
var extensions = ['.yml', '.yaml', '.json'],
+ basename, // everything except the extension
i,
json = false,
raw,
- obj = {};
+ obj;
- if (libpath.extname(filePath)) {
- filePath = filePath.slice(0, libpath.extname(filePath).length * -1);
- }
-
- for (i = extensions.length - 1; i >= 0; i -= 1) {
- try {
- raw = libfs.readFileSync(filePath + extensions[i], 'utf8');
+ obj = this._simpleCache[fullPath];
+ if (!obj) {
+ basename = fullPath;
+ if (libpath.extname(fullPath)) {
+ basename = fullPath.slice(0, libpath.extname(fullPath).length * -1);
+ }
+ for (i = extensions.length - 1; i >= 0; i -= 1) {
try {
- if (i === 2) { // json
- obj = JSON.parse(raw);
- json = true;
- } else { // yaml or yml
- obj = libyaml.load(raw);
- if (json) {
- Y.log(filePath + extensions[2] + ' exists. But ' + extensions[i] + ' file will be used', 'warn', NAME);
+ fullPath = basename + extensions[i];
+ raw = libfs.readFileSync(fullPath, 'utf8');
+ try {
+ if (i === 2) { // json
+ obj = JSON.parse(raw);
+ json = true;
+ } else { // yaml or yml
+ obj = libyaml.load(raw);
+ if (json) {
+ Y.log(basename + extensions[2] + ' exists but ' + extensions[i] + ' file will be used instead', 'warn', NAME);
+ }
}
+ } catch (parseErr) {
+ throw new Error(parseErr);
+ }
+ } catch (err) {
+ if (err.errno !== 34) { // if the error was not "no such file or directory" report it
+ throw new Error("Error parsing file: " + fullPath + "\n" + err);
}
- } catch (parseErr) {
- throw new Error(parseErr);
- }
- } catch (err) {
- if (err.errno !== 34) { // if the error was not "no such file or directory" report it
- throw new Error("Error parsing file: " + filePath + extensions[i] + "\n" + err);
}
}
+ if (!obj) {
+ obj = {};
+ }
+ this._simpleCache[fullPath] = obj;
}
- return obj;
+ return Y.mojito.util.copy(obj);
},
2  lib/app/addons/rs/url.js
View
@@ -116,7 +116,7 @@ YUI.add('addon-rs-url', function(Y, NAME) {
mojitIsPublic = false;
if (mojitRes) {
packageJson = libpath.join(mojitRes.source.fs.fullPath, 'package.json');
- packageJson = store.config.readConfigSimple(packageJson);
+ packageJson = store.config.readConfigJSON(packageJson);
if ('public' === (packageJson.yahoo &&
packageJson.yahoo.mojito &&
packageJson.yahoo.mojito['package'])) {
23 lib/app/autoload/store.server.js
View
@@ -364,6 +364,20 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
return appConfig;
},
+
+
+ /**
+ * Does initial preload of many parts of the application and framework.
+ * The full preload is done by preload().
+ *
+ * @method preloadInitial
+ * @return {nothing}
+ */
+ preloadInitial: function() {
Caridy Patiño Owner
caridy added a note

What happen if preloadInitial and then preload is called? Will be good if we support that use-case, so we can do some stuff before getting to the .preload() execution.

Drew Folta Owner

Yeah, I had thought of that too. I will implement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ this.preloadResourceVersions();
+ },
+
+
/**
* Preloads everything in the app, and as well pertinent parts of
* the framework.
@@ -373,11 +387,14 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
*/
preload: function() {
// We need to do an initial sweep to find the resource store addons.
- this.preloadResourceVersions();
+ this.preloadInitial();
Caridy Patiño Owner
caridy added a note

related to the comment above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
// And then use them.
this.loadAddons();
+
// Then, do another sweep so that the loaded addons can be used.
this.preloadResourceVersions();
+
this.makeResourceVersions();
this.resolveResourceVersions();
},
@@ -1096,7 +1113,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
parents: [],
dir: dir
};
- info.pkg = this.config.readConfigSimple(this._libs.path.join(dir, 'package.json'));
+ info.pkg = this.config.readConfigJSON(this._libs.path.join(dir, 'package.json'));
if (Object.keys(info.pkg).length) {
mojitoVersion = info.pkg.version;
@@ -1794,7 +1811,7 @@ YUI.add('mojito-resource-store', function(Y, NAME) {
} else {
mojitType = this._libs.path.basename(dir);
}
- packageJson = this.config.readConfigSimple(this._libs.path.join(dir, 'package.json'));
+ packageJson = this.config.readConfigJSON(this._libs.path.join(dir, 'package.json'));
if (packageJson) {
if (packageJson.name) {
mojitType = packageJson.name;
27 lib/app/commands/start.js
View
@@ -10,6 +10,7 @@
var path = require('path'),
utils = require(path.join(__dirname, '../../management/utils')),
Mojito = require(path.join(__dirname, '../../mojito')),
+ Store = require(path.join(__dirname, '../../store')),
mojitoVersion = require(path.join(__dirname, '../../../package.json')).version,
fs = require('fs');
@@ -55,31 +56,29 @@ exports.options = [
*/
exports.run = function(params, opts, callback) {
var root = process.cwd(),
+ store,
appConfig,
pack,
inputOptions = opts || {},
options = {},
app;
- try {
- // Are we in a Mojito App? Read the application.json config to find out.
- appConfig = JSON.parse(fs.readFileSync(path.join(root,
- 'application.json'), 'utf8'));
- appConfig = appConfig[0];
- } catch (err) {
- appConfig = {};
+ if (inputOptions.context) {
+ inputOptions.context = utils.contextCsvToObject(inputOptions.context);
}
- try { // Read the package.json config
- pack = JSON.parse(fs.readFileSync(path.join(root, 'package.json'),
- 'utf8'));
- } catch (err2) {
- pack = {};
- }
+ store = Store.createStore({
+ root: root,
+ preload: 'initial',
+ context: inputOptions.context || {}
+ });
+ appConfig = store.getAppConfig();
+
+ pack = store.config.readConfigJSON(path.join(root, 'package.json'));
options.port = params[0] || appConfig.appPort || process.env.PORT || 8666;
Caridy Patiño Owner
caridy added a note

Finally, different port per different context :) nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
if (inputOptions.context) {
- options.context = utils.contextCsvToObject(inputOptions.context);
+ options.context = inputOptions.context;
}
if (inputOptions.perf) {
21 lib/store.js
View
@@ -28,10 +28,11 @@ var Store = {};
/**
* Creates a new server-side resource store instance and returns it.
* @method createStore
- * @param {{root: string,
- * context: Object,
- * appConfig: Object,
- * verbose: boolean}} options An object containing store options.
+ * @param {object} options An object containing store options.
+ * @param {string} options.string Path to application directory.
+ * @param {object} options.context Runtime context to apply to all requests.
+ * @param {object} options.appConfig Overrides for the application.json file.
+ * @param {string "skip"|"initial"|"full"} options.preload Whether to preload the application. Defaults to "full".
* @return Y.mojito.ResourceStore
*/
Store.createStore = function(options) {
@@ -49,6 +50,9 @@ Store.createStore = function(options) {
if (!options.context) {
options.context = {};
}
+ if (!options.preload) {
+ options.preload = 'full';
+ }
// Create a sandboxed YUI instance. This is necessary to avoid shared
// metadata with the main Mojito execution context and the execution context
@@ -79,7 +83,14 @@ Store.createStore = function(options) {
appConfig: options.appConfig
});
- store.preload();
+ switch (options.preload) {
Caridy Patiño Owner
caridy added a note

isn't switch one of the bad parts in javascript? A couple of "if" statements should be just enough considering that the value was already validated/preset.

Caridy Patiño Owner
caridy added a note

Travis failure:

458+ /home/travis/builds/yahoo/mojito/bin/mojito jslint -p
459store.js
460   1 87,9: Expected 'case' at column 5, not column 9.
461     case 'skip': break;
462   2 87,22: Expected 'break' at column 9, not column 22.
463     case 'skip': break;
464   3 88,9: Expected 'case' at column 5, not column 9.
465     case 'initial':
466   4 89,13: Expected 'store' at column 9, not column 13.
467     store.preloadInitial();
468   5 90,13: Expected 'break' at column 9, not column 13.
469     break;
470   6 91,9: Expected 'default' at column 5, not column 9.
471     default:
472   7 92,13: Expected 'store' at column 9, not column 13.
473     store.preload();
474app/commands/start.js
475   1 76,1: Unexpected '(space)'.
Drew Folta Owner

I had no idea switch was contraindicated. The errors are because I forgot to run jslint, which I'll do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ case 'skip': break;
+ case 'initial':
+ store.preloadInitial();
+ break;
+ default:
+ store.preload();
+ }
return store;
};
36 tests/func/applications/frameworkapp/yaml-config/application.json
View
@@ -1,23 +1,17 @@
[
- {
- "settings": [ "master" ],
- "log":{
- "client":{
- "level":"debug",
- "yui":true,
- "timestamp": false
- },
- "server":{
- "level":"debug",
- "yui":true,
- "timestamp": true
- }
- },
- "appPort": 4001,
- "specs": {
- "simple": {
- "type": "simple"
- }
+ {
+ "settings": [ "master" ],
+ "yui": {
+ "config": {
+ "debug": true,
+ "logLevel": "debug"
+ }
+ },
+ "appPort": 4001,
+ "specs": {
+ "simple": {
+ "type": "simple"
+ }
+ }
}
- }
-]
+]
9 tests/func/applications/frameworkapp/yaml-config/application.yaml
View
@@ -6,9 +6,8 @@
settings: [master]
# Port number
appPort: 4000
- # Set the log level for the application
- log:
- # On the server only
- server:
+ yui:
+ config:
+ debug: true
# This is the level that will be used
- level: debug
+ logLevel: debug
2  tests/func/applications/frameworkapp/yaml-config/mojits/simple/controller.common.js
View
@@ -10,4 +10,6 @@ YUI.add("simple-ctrl", function (Y, NAME) {
ac.done(ac.config.get("message"));
}
};
+}, '0.0.1', {
+ requires: ['mojito-config-addon']
});
10 ...l-config/yamlconfigtest_descrip-DISABLED-tor.json → ...s/func/yaml-config/yamlconfigtest_descriptor.json
View
@@ -1,6 +1,3 @@
-
-see trello mojito-perf card #204
-
[
{
"settings": [ "master" ],
@@ -8,7 +5,12 @@ see trello mojito-perf card #204
"name" : "yaml-config",
"config" :{
- "baseUrl" : "http://localhost:4000"
+ "baseUrl" : "http://localhost:8666",
+ "application" : {
+ "name":"yaml-config",
+ "path": "./frameworkapp/yaml-config",
+ "type": "mojito"
+ }
},
"dataprovider" : {
61 tests/unit/lib/app/addons/rs/test-config.js
View
@@ -212,18 +212,6 @@ YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
},
- 'read JSON files': function() {
- var fixtures = libpath.join(__dirname, '../../../../../fixtures/store');
- var store = new MockRS({ root: fixtures });
- store.plug(Y.mojito.addons.rs.config, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
-
- var path = libpath.join(fixtures, 'application.json');
- var have = store.config.readConfigSimple(path);
- var want = readJSON(fixtures, 'application.json');
- cmp(have, want);
- },
-
-
'read YCB files': function() {
var fixtures = libpath.join(__dirname, '../../../../../fixtures/store');
var store = new MockRS({ root: fixtures });
@@ -288,9 +276,10 @@ YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
var path = libpath.join(fixtures, 'routes.json');
try {
store.config.readConfigSimple(path);
+ A.fail("should throw an error");
}
catch (err) {
- A.areSame('Error parsing JSON file:', err.message.substr(0, 24));
+ A.areSame('Error parsing file:', err.message.substr(0, 19));
}
},
@@ -304,30 +293,33 @@ YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
var have = store.config.readConfigYCB(path, {});
var want = {};
cmp(have, want);
- }
+ },
-
- }));
-
- suite.add(new YUITest.TestCase({
-
- name: 'config rs addon tests',
- "readConfigSync JSON file": function () {
+ "readConfigJSON JSON file": function() {
+ var fixtures = libpath.join(__dirname, '../../../../../fixtures/gsg5');
+ var store = new MockRS({ root: fixtures });
+ store.plug(Y.mojito.addons.rs.config, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
+
+ var path = libpath.join(fixtures, 'package.json');
+ var have = store.config.readConfigJSON(path);
+ var want = readJSON(fixtures, 'package.json');
+ cmp(have, want);
+ },
+
+ "readConfigSimple JSON file": function () {
var fixtures = libpath.join(__dirname, '../../../../../fixtures'),
store = new MockRS({ root: fixtures }),
path = libpath.join(fixtures, "/config/", "json.json"),
obj;
-
store.plug(Y.mojito.addons.rs.config, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
-
- obj = store.config.readConfigSync(path);
-
+ obj = store.config.readConfigSimple(path);
A.areSame("val", obj.key);
},
- "readConfigSync YAML file": function () {
+
+ "readConfigSimple YAML file": function () {
var fixtures = libpath.join(__dirname, '../../../../../fixtures'),
store = new MockRS({ root: fixtures }),
@@ -336,12 +328,13 @@ YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
store.plug(Y.mojito.addons.rs.config, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
- obj = store.config.readConfigSync(path);
+ obj = store.config.readConfigSimple(path);
A.areSame("val", obj.key);
},
- "readConfigSync YML file": function () {
+
+ "readConfigSimple YML file": function () {
var fixtures = libpath.join(__dirname, '../../../../../fixtures'),
store = new MockRS({ root: fixtures }),
@@ -350,12 +343,13 @@ YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
store.plug(Y.mojito.addons.rs.config, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
- obj = store.config.readConfigSync(path);
+ obj = store.config.readConfigSimple(path);
A.areSame("val", obj.key);
},
- "readConfigSync no ext file": function () {
+
+ "readConfigSimple no ext file": function () {
var fixtures = libpath.join(__dirname, '../../../../../fixtures'),
store = new MockRS({ root: fixtures }),
@@ -364,12 +358,13 @@ YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
store.plug(Y.mojito.addons.rs.config, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
- obj = store.config.readConfigSync(path);
+ obj = store.config.readConfigSimple(path);
A.areSame("val", obj.key);
},
- "readConfigSync YAML file with TAB not space": function () {
+
+ "readConfigSimple YAML file with TAB not space": function () {
var fixtures = libpath.join(__dirname, '../../../../../fixtures'),
store = new MockRS({ root: fixtures }),
@@ -378,7 +373,7 @@ YUI().use('addon-rs-config', 'mojito-util', 'base', 'oop', 'test', function(Y) {
store.plug(Y.mojito.addons.rs.config, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
try {
- store.config.readConfigSync(path);
+ store.config.readConfigSimple(path);
}
catch (err) {
A.areSame('Error parsing file:', err.message.substr(0, 19));
2  tests/unit/lib/app/addons/rs/test-url.js
View
@@ -28,7 +28,7 @@ YUI().use('addon-rs-url', 'base', 'oop', 'test', function(Y) {
this._mojitRVs = {};
this.publish('getMojitTypeDetails', {emitFacade: true, preventable: false});
this.config = {
- readConfigSimple: function() { return {} }
+ readConfigJSON: function() { return {} }
};
},
2  tests/unit/lib/app/addons/rs/test-yui.js
View
@@ -363,7 +363,7 @@ YUI().use(
store = new Y.mojito.ResourceStore({ root: fixtures });
// fake out some parts of preload(), which we're trying to avoid
- store._fwConfig = store.config.readConfigSimple(libpath.join(mojitoRoot, 'config.json'));
+ store._fwConfig = store.config.readConfigJSON(libpath.join(mojitoRoot, 'config.json'));
store._appConfigStatic = store.getStaticAppConfig();
store.plug(Y.mojito.addons.rs.yui, { appRoot: fixtures, mojitoRoot: mojitoRoot } );
2  tests/unit/lib/app/autoload/test-store.server.js
View
@@ -548,7 +548,7 @@ YUI().use(
store = new Y.mojito.ResourceStore({ root: fixtures });
// fake out some parts of preload(), which we're trying to avoid
- store._fwConfig = store.config.readConfigSimple(libpath.join(mojitoRoot, 'config.json'));
+ store._fwConfig = store.config.readConfigJSON(libpath.join(mojitoRoot, 'config.json'));
store._appConfigStatic = store.getStaticAppConfig();
var dir = libpath.join(__dirname, '../../../../fixtures/conventions');
Something went wrong with that request. Please try again.