Skip to content

Commit f848f7b

Browse files
pilsypilsy
committed
fix(autoRouting): Fixed bug with autoRouting and expanded to allow use without clever-injector
chore(cleanup): Fixed indentation etc
1 parent 725e5e4 commit f848f7b

2 files changed

Lines changed: 135 additions & 82 deletions

File tree

‎controller.js‎

Lines changed: 106 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33

44
var NoActionException = require('./exceptions/NoAction')
55
, Class = require( 'uberclass' )
6-
, path = require( 'path' );
6+
, path = require( 'path' )
7+
, util = require( 'util' )
8+
, i = require( 'i' )()
9+
, debug = require( 'debug' )( 'clever-controller' )
10+
, routedControllers = [];
711

812
module.exports = Class.extend(
913
/* @Static */
@@ -22,57 +26,92 @@ module.exports = Class.extend(
2226

2327
setup: function() {
2428
var self = this;
25-
if ( typeof injector !== 'undefined' && this.autoRouting !== false && this.route !== false ) {
26-
injector.inject( function( app ) {
27-
self.autoRoute( app );
28-
});
29+
if ( this.autoRouting !== false && this.route !== false && routedControllers.indexOf( this.route ) === -1 ) {
30+
// Do not route multiple times
31+
routedControllers.push( this.route );
32+
33+
if ( typeof this.app !== 'undefined' ) {
34+
35+
// app has been provided for us in the Static of this Controller so we can attach our routes to it
36+
this.autoRoute( this.app );
37+
38+
} else if ( typeof injector !== 'undefined' ) {
39+
40+
// Use the clever-injector to get the express app so we can attach our routes to it
41+
injector.inject( function( app ) {
42+
self.autoRoute( app );
43+
});
44+
45+
}
2946
}
3047
},
3148

3249
autoRoute: function( app ) {
33-
var middleware = [];
50+
var middleware = []
51+
, routes = this.route.split( '|' )
52+
53+
debug( 'Autorouting for route ' + routes.join( ', ' ) );
3454

55+
// Check for middleware that we need to put before the actual attach()
3556
if ( this.autoRouting instanceof Array ) {
57+
debug( 'Found middleware for ' + routes.join( ', ' ) + ' - ' + util.inspect( this.autoRouting ).replace( /\n/ig, ' ' ) );
58+
3659
this.autoRouting.forEach(function( mw ) {
3760
middleware.push( typeof mw === 'string' ? this.callback( mw ) : mw );
3861
}.bind( this ));
3962
}
4063

64+
// Add our attach() function
4165
middleware.push( this.attach() );
4266

43-
app.all.apply( app, [ [ this.route, ':action', ':id?' ].join( '/' ) ].concat( middleware ) ); // /example/:action/:id?
44-
app.all.apply( app, [ [ this.route, ':action?' ].join( '/' ) ].concat( middleware ) ); // /example/?:action?
67+
// Bind the actual routes
68+
routes.forEach(function( route ) {
69+
var actionIdRoute = [ route, ':action', ':id?' ].join( '/' )
70+
, actionRoute = [ route, ':action?' ].join( '/' );
71+
72+
debug( 'Attaching route ' + actionIdRoute );
73+
app.all.apply( app, [ actionIdRoute ].concat( middleware ) ); // /example/:action/:id?
74+
75+
debug( 'Attaching route ' + actionRoute );
76+
app.all.apply( app, [ actionRoute ].concat( middleware ) ); // /example/?:action?
77+
});
4578
},
4679

4780
extend: function() {
4881
var extendingArgs = [].slice.call( arguments )
4982
, autoRouting = ( extendingArgs.length === 2 )
5083
? extendingArgs[ 0 ].autoRouting !== false
51-
: false
84+
: this.autoRouting
5285
, definedRoute = ( extendingArgs.length === 2 )
5386
? extendingArgs[ 0 ].route !== undefined
54-
: false;
87+
: this.route;
5588

89+
// Figure out if we are autoRouting and do not have a defined route already
5690
if ( autoRouting && !definedRoute ) {
5791
var stack = new Error().stack.split( '\n' )
58-
, stack = stack.splice( 1, stack.length - 1)
92+
, stack = stack.splice( 1, stack.length - 1 )
5993
, extendingFilePath = false
6094
, extendingFileName = false
6195
, route = null;
6296

97+
// Walk backwards over the stack to find the filename where this is defined
6398
while( stack.length > 0 && extendingFilePath === false ) {
6499
var file = stack.shift();
65100
if ( !/clever-controller/ig.test( file ) && !/uberclass/ig.test( file ) ) {
66101
if ( /\(([^\[\:]+).*\)/ig.test( file ) ) {
67102
extendingFilePath = RegExp.$1;
68-
extendingFileName = path.basename( extendingFilePath ).replace( /(controller)?.js/ig, '' ).toLowerCase();
103+
extendingFileName = path.basename( extendingFilePath );
69104
}
70105
}
71106
}
72107

73-
if ( [ '', 'controller' ].indexOf( extendingFileName ) === -1 && this.route === false ) {
74-
route = [ '/', extendingFileName ].join('');
75-
// debug( 'Binding automatic route name??' )
108+
// Determine the route names if we have found a file
109+
if ( [ '', 'controller.js' ].indexOf( extendingFileName.toLowerCase() ) === -1 ) {
110+
var singular = i.singularize( extendingFileName.replace( /(controller)?.js/ig, '' ).toLowerCase() )
111+
, plural = i.pluralize( singular );
112+
113+
route = [ '/', singular, '|', '/', plural ].join('');
114+
76115
if ( extendingArgs.length === 2 ) {
77116
extendingArgs[ 0 ].route = route;
78117
} else {
@@ -81,7 +120,7 @@ module.exports = Class.extend(
81120
}
82121
}
83122

84-
123+
// Call extend on the parent
85124
return this._super.apply( this, extendingArgs );
86125
}
87126
},
@@ -95,115 +134,128 @@ module.exports = Class.extend(
95134

96135
setup: function(req, res, next) {
97136
try {
98-
return this.performanceSafeSetup(req, res, next);
99-
} catch(e) {
100-
return [e];
137+
return this.performanceSafeSetup( req, res, next );
138+
} catch( e ) {
139+
return [ e ];
101140
}
102141
},
103142

104-
performanceSafeSetup: function(req, res, next) {
105-
var method = null,
106-
funcName = null;
143+
performanceSafeSetup: function( req, res, next ) {
144+
var method = null
145+
, funcName = null
146+
, parts = null;
107147

108148
this.next = next;
109149
this.req = req;
110150
this.res = res;
111151

112152
// Override routes where you attach specifically to a single route
113-
if (this.Class.actionRouting && /\//.test(this.req.url)) {
114-
var parts = this.req.url.split('/');
115-
funcName = parts[parts.length-1];
153+
if ( this.Class.actionRouting && /\//.test( this.req.url ) ) {
154+
parts = this.req.url.split( '/' );
155+
funcName = parts[ parts.length - 1 ];
116156

117157
if(/\#|\?/.test(funcName)){
118-
funcName = funcName.split(/\#|\?/)[0];
158+
funcName = funcName.split( /\#|\?/ )[ 0 ];
119159
}
120160

121-
if (isNaN(funcName)) {
161+
if ( isNaN( funcName ) ) {
122162
funcName = funcName + 'Action';
123-
if (typeof this[funcName] == 'function') {
124-
return [null, funcName, next];
163+
if ( typeof this[ funcName ] == 'function' ) {
164+
debug( 'actionRouting mapped to ' + funcName );
165+
166+
return [ null, funcName, next ];
125167
}
126168
}
127169
}
128170

129171
// Route based on an action first if we can
130-
if (this.Class.actionRouting && typeof this.req.params !== 'undefined' && typeof this.req.params.action !== 'undefined') {
172+
if ( this.Class.actionRouting && typeof this.req.params !== 'undefined' && typeof this.req.params.action !== 'undefined' ) {
131173
// Action Defined Routing
132-
if (isNaN(this.req.params.action)) {
174+
if ( isNaN( this.req.params.action ) ) {
133175
funcName = this.req.params.action + 'Action';
134176

135-
if (typeof this[funcName] == 'function') {
136-
return [null, funcName, next];
177+
if ( typeof this[ funcName ] == 'function' ) {
178+
debug( 'actionRouting mapped to ' + funcName );
179+
return [ null, funcName, next ];
137180
} else {
138181
throw new NoActionException();
139182
}
140183
} else {
141184
// HTTP Method Based Routing
142185
method = this.req.method.toLowerCase() + 'Action';
143-
if (typeof this[method] == 'function') {
186+
if ( typeof this[ method ] == 'function' ) {
187+
debug( 'http method route mapped to ' + method );
144188

145189
this.req.params.id = this.req.params.action;
146190
delete this.req.params.action;
147191

148-
return [null, method, next];
192+
return [ null, method, next ];
149193
} else {
150194
throw new NoActionException();
151195
}
152196
}
153197
}
154198

155199
// Route based on the HTTP Method, otherwise throw an exception
156-
if (this.Class.restfulRouting) {
157-
if (this.isGet() && (this.req.params === undefined || this.req.params.id === undefined) && typeof this.listAction === 'function') {
200+
if ( this.Class.restfulRouting ) {
201+
if ( this.isGet() && ( this.req.params === undefined || this.req.params.id === undefined ) && typeof this.listAction === 'function' ) {
158202
method = 'listAction';
203+
204+
debug( 'restfulRouting mapped to ' + method );
159205
} else {
160206
method = this.req.method.toLowerCase() + 'Action';
161-
if (typeof this[method] != 'function') {
207+
if ( typeof this[ method ] != 'function' ) {
162208
throw new NoActionException();
163209
}
210+
211+
debug( 'restfulRouting mapped to ' + method );
164212
}
165213
}
166214

167215
// If we got this far without an action but with a method, then route based on that
168-
return [null, method, next];
216+
return [ null, method, next ];
169217
},
170218

171-
init: function(error, method, next) {
172-
if (error && error instanceof NoActionException) {
219+
init: function( error, method, next ) {
220+
if ( error && error instanceof NoActionException ) {
221+
debug( 'No route mapping found, calling next()' );
222+
173223
this.next();
174224
} else {
175225
try {
176-
if (error)
226+
if ( error )
177227
throw error;
178228

179-
if (method !== null) {
229+
if ( method !== null ) {
180230
this.action = method;
181-
this[method](this.req, this.res);
231+
232+
debug( 'calling ' + this.action );
233+
this[ method ]( this.req, this.res );
182234
} else {
183235
this.next();
184236
}
185237

186-
} catch(e) {
187-
this.handleException(e);
238+
} catch( e ) {
239+
this.handleException( e );
188240
}
189241
}
190242
},
191243

192-
send: function(content, code, type) {
244+
send: function( content, code, type ) {
193245
var toCall = type || this.resFunc;
194-
if (code) {
195-
this.res[toCall](code, content);
246+
if ( code ) {
247+
this.res[ toCall ]( code, content );
196248
} else {
197-
this.res[toCall](content);
249+
this.res[ toCall ]( content );
198250
}
199251
},
200252

201-
render: function(template, data) {
202-
this.res.render(template, data);
253+
render: function( template, data ) {
254+
this.res.render( template, data );
203255
},
204256

205-
handleException: function(exception) {
206-
this.send({ error: 'Unhandled exception: ' + exception, stack: exception.stack }, 500);
257+
handleException: function( exception ) {
258+
this.send( { error: 'Unhandled exception: ' + exception, stack: exception.stack }, 500 );
207259
},
208260

209261
isGet: function() {

‎package.json‎

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,31 @@
11
{
2-
"name": "clever-controller",
3-
"description": "Lightning-fast flexible controller prototype",
4-
"version": "1.1.1",
5-
"private": false,
6-
"repository":
7-
{
8-
"type" : "git",
9-
"url" : "http://github.com/CleverStack/clever-controller.git"
10-
},
11-
"scripts": {
12-
"test": "mocha --require should --reporter spec"
13-
},
14-
"dependencies": {
15-
"uberclass": ""
16-
},
17-
"devDependencies": {
18-
"mocha": "*",
19-
"should": "*",
20-
"sinon": "*"
21-
},
22-
"author": {
23-
"name": "Richard Gustin (PilsY)",
24-
"email": "richard@clevertech.biz"
25-
},
26-
"keywords": [
27-
"controller",
28-
"cleverstack"
29-
]
2+
"name": "clever-controller",
3+
"description": "Lightning-fast flexible controller prototype",
4+
"version": "1.1.2",
5+
"private": false,
6+
"repository": {
7+
"type": "git",
8+
"url": "http://github.com/CleverStack/clever-controller.git"
9+
},
10+
"scripts": {
11+
"test": "mocha --require should --reporter spec"
12+
},
13+
"dependencies": {
14+
"debug": "^0.8.1",
15+
"i": "^0.3.2",
16+
"uberclass": ""
17+
},
18+
"devDependencies": {
19+
"mocha": "*",
20+
"should": "*",
21+
"sinon": "*"
22+
},
23+
"author": {
24+
"name": "Richard Gustin (PilsY)",
25+
"email": "richard@clevertech.biz"
26+
},
27+
"keywords": [
28+
"controller",
29+
"cleverstack"
30+
]
3031
}

0 commit comments

Comments
 (0)