Skip to content

Commit

Permalink
Use the utilities available in the global geddy object rather than re…
Browse files Browse the repository at this point in the history
…-requiring utilities in nearly every file.
  • Loading branch information
larzconwell committed Oct 19, 2012
1 parent 95f02ea commit ed0158e
Show file tree
Hide file tree
Showing 15 changed files with 72 additions and 74 deletions.
3 changes: 1 addition & 2 deletions bin/cli.js
Expand Up @@ -4,7 +4,6 @@
var geddy = require('../lib/geddy')
, exec = require('child_process').exec
, path = require('path')
, utils = require('utilities')
, parseopts = require('../lib/parseopts');

// Variables
Expand Down Expand Up @@ -223,7 +222,7 @@ if (cmds.length) {
// Just `geddy` -- start the server
else {
// Search for 'config' directory in parent directories
utils.file.searchParentPath('config', function (err, filePath) {
geddy.file.searchParentPath('config', function (err, filePath) {
if (err) {
die(usage);
}
Expand Down
1 change: 0 additions & 1 deletion lib/app.js
Expand Up @@ -25,7 +25,6 @@ var fs = require('fs')
, cwd = process.cwd()
, errors = require('./response/errors')
, response = require('./response')
, utils = require('utilities')
, init = require('./init')
, helpers = require('./template/helpers')
, actionHelpers = require('./template/helpers/action')
Expand Down
2 changes: 1 addition & 1 deletion lib/base_config.js
@@ -1,4 +1,4 @@
var path = require('path')
var path = require('path')
, config
, cwd = process.cwd();

Expand Down
25 changes: 11 additions & 14 deletions lib/cluster/master.js
Expand Up @@ -4,10 +4,7 @@ var Master
, path = require('path')
, Log = require('../../deps/log')
, dispatch = require('./master_dispatch')
, config = require('../config')
, utils = require('utilities')
, file = utils.file
, date = utils.date;
, config = require('../config');

var processModes = {
KEEP_ALIVE: 'keepAlive'
Expand Down Expand Up @@ -43,7 +40,7 @@ Master.prototype = new (function () {
, _initLogging = function (next) {
var self = this
, types = ['stdout', 'stderr', 'access']
, now = date.strftime(new Date(), '%FT%T')
, now = geddy.date.strftime(new Date(), '%FT%T')
, dir = this.config.logDir
, levelsByType = {access: 'access', stderr: 'error', stdout: 0}
, writing = dir != null
Expand Down Expand Up @@ -73,22 +70,22 @@ Master.prototype = new (function () {
// If we are writing, then attempt to create the directory
// if it doesn't exist
if (writing) {
file.mkdirP(dir);
geddy.file.mkdirP(dir);
}

types.forEach(function (type) {
var currentLog = path.join(dir, type + '.log')
, archivedLog = path.join(dir, type + '.' + now + '.log');

// If the main log file exists, then rename it to the archived log file
if (writing && file.existsSync(currentLog)) {
if (writing && geddy.file.existsSync(currentLog)) {
try {
fs.renameSync(currentLog, archivedLog);
}
// renameSync doesn't work correctly here for some reason,
// fall back to copy/delete
catch (e) {
file.cpR(currentLog, archivedLog);
geddy.file.cpR(currentLog, archivedLog);
fs.unlinkSync(currentLog);
}
}
Expand Down Expand Up @@ -116,7 +113,7 @@ Master.prototype = new (function () {
, port;
if (this.config.metrics) {
port = this.config.metrics.port;
metrics = file.requireLocal('metrics');
metrics = geddy.file.requireLocal('metrics');
this.stdoutLog.info('Metrics server started on port ' + port);
this.metricsServer = new metrics.Server(port);
}
Expand Down Expand Up @@ -211,11 +208,11 @@ Master.prototype = new (function () {
};
// Watch individual files so we can compare mtimes and restart
// on code-changes
file.watch(dir + '/config', callback);
file.watch(dir + '/lib', callback);
file.watch(dir + '/app/controllers', callback);
file.watch(dir + '/app/models', callback);
file.watch(dir + '/app/views', callback);
geddy.file.watch(dir + '/config', callback);
geddy.file.watch(dir + '/lib', callback);
geddy.file.watch(dir + '/app/controllers', callback);
geddy.file.watch(dir + '/app/models', callback);
geddy.file.watch(dir + '/app/views', callback);
};

this.init = function () {
Expand Down
5 changes: 2 additions & 3 deletions lib/cluster/worker.js
@@ -1,5 +1,4 @@
var fs = require('fs')
, utils = require('utilities')
, dispatch = require('./worker_dispatch')
, Logger = require('./worker_logger').Logger
, Worker;
Expand Down Expand Up @@ -52,7 +51,7 @@ Worker.prototype = new (function () {
// If SPDY options were given
else if (spdy) {
if (spdy.cert && spdy.key) {
this.server = utils.file.requireLocal('spdy').createServer({
this.server = geddy.file.requireLocal('spdy').createServer({
key: fs.readFileSync(spdy.key)
, cert: fs.readFileSync(spdy.cert)
});
Expand All @@ -76,7 +75,7 @@ Worker.prototype = new (function () {
, ssl = this.config.ssl ? ' (SSL)' : ''
, spdy = this.config.spdy ? '(SPDY)' : '';

utils.network.isPortOpen(port, hostname, function (err, isOpen) {
geddy.network.isPortOpen(port, hostname, function (err, isOpen) {
if (err) {
self.log.error(err);
}
Expand Down
29 changes: 21 additions & 8 deletions lib/geddy.js
@@ -1,10 +1,27 @@
/*
* Geddy JavaScript Web development framework
* Copyright 2112 Matthew Eernisse (mde@fleegix.org)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/

var geddy = global.geddy || {}
, cluster = require('cluster')
, utils = require('./utils')
, pkg = require('../package')
, master
, worker
, utils = require('utilities')
, pkg = require('../package')
, App;

geddy.isMaster = cluster.isMaster;
Expand Down Expand Up @@ -57,9 +74,5 @@ utils.mixin(geddy, new (function () {

})());

// Set the One True Geddy Global
global.geddy = geddy;

// Also allow export/local
module.exports = geddy;

// Set geddy as global and as an export/local
global.geddy = module.exports = geddy;
13 changes: 5 additions & 8 deletions lib/init/i18n.js
@@ -1,8 +1,5 @@
var path = require('path')
, fs = require('fs')
, utils = require('utilities')
, i18n = utils.i18n
, file = utils.file;
, fs = require('fs');

module.exports = new (function () {
var LOCALE_PAT = /([^\/]*).json$/;
Expand All @@ -14,9 +11,9 @@ module.exports = new (function () {
, loadLocaleData = function () {
localePaths.forEach(function (directory) {
directory = path.normalize(directory);
if (file.existsSync(directory)) {
if (geddy.file.existsSync(directory)) {
var f
, files = file.readdirR(directory);
, files = geddy.file.readdirR(directory);
for (var i = 0; i < files.length; i++) {
f = files[i];
if (f && /.json$/.test(f)) {
Expand All @@ -31,7 +28,7 @@ module.exports = new (function () {
throw new Error('Could not parse locale-data in file: ' +
f);
}
i18n.loadLocale(locale[1], data);
geddy.i18n.loadLocale(locale[1], data);
}
}
}
Expand All @@ -41,7 +38,7 @@ module.exports = new (function () {
};

localePaths = localePaths.concat(geddy.config.i18n.loadPaths || []);
i18n.setDefaultLocale(geddy.config.i18n.defaultLocale);
geddy.i18n.setDefaultLocale(geddy.config.i18n.defaultLocale);
loadLocaleData();
};

Expand Down
32 changes: 16 additions & 16 deletions lib/init/index.js
@@ -1,25 +1,25 @@

// Look through the './init' directory, and run the `init`
// method for all listed files -- each `init` method should
// take a callback that runs the next one. When all finished
// call the original top-level callback for the whole init
// process
exports.init = function (app, callback) {
var items = [
'router'
, 'model'
, 'i18n'
, 'helpers'
]
, initItem = function () {
var item = items.shift();
if (item) {
item = require('./' + item);
item.init(app, initItem);
}
else {
callback();
}
};
'router'
, 'model'
, 'i18n'
, 'helpers'
];

var initItem = function () {
var item = items.shift();
if (item) {
item = require('./' + item);
item.init(app, initItem);
}
else {
callback();
}
};
initItem();
};
15 changes: 7 additions & 8 deletions lib/init/model.js
@@ -1,23 +1,22 @@
var model = require('model')
, path = require('path')
, utils = require('utilities')
, useCoffee = false;

module.exports = new (function () {

var _getAdapterPath = function (base, name) {
var p = path.join(base, utils.string.snakeize(name))
var p = path.join(base, geddy.string.snakeize(name))
, testPath;

// Fucking CoffeeScript
testPath = p + '.coffee';
if (utils.file.existsSync(testPath)) {
useCoffee = useCoffee || utils.file.requireLocal('coffee-script');
if (geddy.file.existsSync(testPath)) {
useCoffee = useCoffee || geddy.file.requireLocal('coffee-script');
return testPath;
}
// Normal JS
testPath = p + '.js';
if (utils.file.existsSync(testPath)) {
if (geddy.file.existsSync(testPath)) {
return testPath;
}
// Nothing found
Expand Down Expand Up @@ -45,14 +44,14 @@ module.exports = new (function () {
, ctors;

// May be running totally model-less
if (!utils.file.existsSync(path.join(cwd, modelDir))) {
if (!geddy.file.existsSync(path.join(cwd, modelDir))) {
return callback();
}

// Set any model properties from the app config
utils.mixin(model, geddy.config.model);
geddy.mixin(model, geddy.config.model);

models = utils._getConstructorsFromDirectory(modelDir);
models = geddy._getConstructorsFromDirectory(modelDir);
models.forEach(function (m) {
var name = m.ctorName
, filePath = m.filePath
Expand Down
5 changes: 2 additions & 3 deletions lib/init/router.js
@@ -1,5 +1,4 @@
var path = require('path')
, utils = require('utilities')
, FunctionRouter = require('../routers/function_router').FunctionRouter
, RegExpRouter = require('barista').Router;

Expand All @@ -13,8 +12,8 @@ module.exports = new (function () {
, routerCsFile = path.join(cwd, '/config/router.coffee')
, router;

if (utils.file.existsSync(routerCsFile)) {
utils.file.requireLocal('coffee-script');
if (geddy.file.existsSync(routerCsFile)) {
geddy.file.requireLocal('coffee-script');
}
router = require(path.join(cwd, '/config/router'));
router = router.router || router;
Expand Down
3 changes: 1 addition & 2 deletions lib/response/format.js
@@ -1,5 +1,4 @@
var utils = require('utilities')
, format
var format
, Format
, builtInFormats;

Expand Down
3 changes: 1 addition & 2 deletions lib/sessions/stores/memcache.js
Expand Up @@ -28,8 +28,7 @@ Config section should look like this:
'servers' is your array of Memcache servers.
*/

var file = require('utilities').file
, Memcached = file.requireLocal('memcached');
var Memcached = geddy.file.requireLocal('memcached');

var Memcache = function (callback) {
this.setup(callback);
Expand Down
3 changes: 1 addition & 2 deletions lib/sessions/stores/mongodb.js
Expand Up @@ -36,8 +36,7 @@ Config section should look like this:
If don't provide an expiry in your config file your sessions will live forever.
*/
var file = require('utilities').file
, mongo = file.requireLocal('mongodb-wrapper');
var mongo = geddy.file.requireLocal('mongodb-wrapper');

var MongoDB = function (callback) {
this.setup(callback);
Expand Down
3 changes: 1 addition & 2 deletions lib/sessions/stores/redis.js
Expand Up @@ -33,8 +33,7 @@ Config section should look like this:
'server' is your Redis server.
*/
var file = require('utilities').file
, RedisServer = file.requireLocal('redis');
var RedisServer = geddy.file.requireLocal('redis');

var Redis = function (callback) {
this.setup(callback);
Expand Down
4 changes: 2 additions & 2 deletions lib/template/index.js
Expand Up @@ -197,14 +197,14 @@ getTemplateData = function (templateRoot, partialURL, parentNode, isLayout) {
// If default layout doesn't exist we have no other layout so throw error
else {
err = new geddy.errors.InternalServerError('Layout template "' +
partialURL + '" not found in ' + geddy.utils.array.humanize(dirs));
partialURL + '" not found in ' + geddy.array.humanize(dirs));
throw err;
}
}
// If it's a normal template then it doesn't exist
else {
err = new geddy.errors.InternalServerError('Partial template "' +
partialURL + '" not found in ' + geddy.utils.array.humanize(dirs));
partialURL + '" not found in ' + geddy.array.humanize(dirs));
throw err;
}
}
Expand Down

4 comments on commit ed0158e

@mde
Copy link
Contributor

@mde mde commented on ed0158e Oct 21, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a tough one. While I think it's a good thing that we hang some useful stuff like "log" and various utilities onto the geddy global for people building apps with Geddy, I don't think we should be depending heavily on non-explicit, magical globals throughout the framework code. This makes it harder to pull these modules apart for testing, and sometimes order-of-loading issues mean stuff isn't on the global when you expect it, and things break.

I've been slowly removing this dependence on the "geddy" global in favor of explicit requires in our framework code, which will make it way easier to write tests. I'm going to have to revert this commit, man.

Might be a good idea too, if you are going to make a large, sweeping change, to float a balloon on the mailing list, or open a pull-request for yourself, to start a quick discussion about it.

@kieran
Copy link
Contributor

@kieran kieran commented on ed0158e Oct 21, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see the benefit of a global app log instance since it's a singleton, but I see no disadvantage to per-module or per-file requires for most anything else. Node caches them internally, so there's virtually no performance hit afaik.

Logs are kinda funny as well, since there's both an app log and a request log. Are we differentiating there?

@larzconwell
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense @mde, thanks for explaining!

@mde
Copy link
Contributor

@mde mde commented on ed0158e Oct 22, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kieran Hanging various utilities on the global "geddy" object, comes mostly from the paucity of those in plain JS, and the inability to add stuff to the built-ins. (We're not taking the Prototype route here.) Most people eventually want stuff like string.camelize or date.strftime, and providing those as built-in is a nicety I think a framework like Geddy should provide. (In my mind this is similar to the Railsy String.blank.)

As far as the logs go -- there are a bunch of different log-levels available. Basically 'access' goes to the access-log, and all the other levels go either to the stdout or the stderr logs. This might be a little weirder now too since I've just pushed changes to master which will make it easier for people to run their Geddy app in a single process, and allows you to pass in a custom logger.

Please sign in to comment.