refactor routeParams and params property creation to be done in the r…

…equest constructor move logic to get request parameters for a route into a method on the route
nrstott · Feb 6, 2013 · b6f8a9c · b6f8a9c
1 parent 363faac
commit b6f8a9c
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 20 deletions.
diff --git a/lib/request.js b/lib/request.js
@@ -13,6 +13,15 @@ module.exports = function(router, jsgiReq) {
       get: function() {
         return !util.no(this.headers['x-requested-with']);
       }
+    },
+    routeParams: {
+      value: {},
+      enumerable: true
+    },
+    params: {
+      get: function() {
+        return util.merge({}, this.routeParams, this.search, this.body);
+      }
     }
   });
 };
diff --git a/lib/router.js b/lib/router.js
@@ -145,7 +145,7 @@ Router.prototype.route = function(method, path /*, handlers... */) {
     path = new RegExp("^"+path.replace(/\./, '\\.').replace(/\*/g, '(.+)').replace(PATH_PARAMETERS, PATH_PARAMETER_REPLACEMENT)+'$');
   }
 
-  route = { path: path, paramNames: paramNames, apps: apps, originalPath: originalPath };
+  route = makeRoute({ path: path, paramNames: paramNames, apps: apps, originalPath: originalPath });
 
   this.emit(exports.bogartEvent.BEFORE_ADD_ROUTE, this, route);
 
@@ -182,31 +182,13 @@ Router.prototype.respond = function(reqPromise) {
 
   return when(reqPromise, function(req) {
     var route = self.handler(req.method, req.pathInfo)
-      , routeParams = {}
       , routeParamValues = null;
 
     if (util.no(route)) {
       return null;
     }
 
-    routeParamValues = route.path.exec(req.pathInfo);
+    routeParamValues = route.getParamValuesFromRequest(req);
-    if (routeParamValues) {
-      routeParamValues.shift(); // Remove the initial match
-
-      routeParamValues.forEach(function(val, indx) {
-        val = decodeURIComponent(val);
-        if (route.paramNames && route.paramNames.length > indx) {
-          routeParams[route.paramNames[indx]] = val;
-        } else if (val !== undefined) {
-          routeParams.splat = routeParams.splat || [];
-          routeParams.splat.push(val);
-        }
-      });
-    }
-
-    Object.defineProperty(req, 'routeParams', { value: routeParams, enumerable: true, readonly: true });
-
-    Object.defineProperty(req, 'params', { value: util.merge({}, req.routeParams, req.search, req.body), enumerable: true, readonly: true });
 
     /* Invoke all 'before' functions and save any promises. */
     var promiseArray = [];
@@ -293,4 +275,30 @@ function thenResolve(val) {
   return function() {
     return val;
   }
+}
+
+function makeRoute(proto) {
+  return Object.create(proto, {
+    getParamValuesFromRequest: {
+      value: function(req) {
+        var route = this
+          , routeParamValues = route.path.exec(req.pathInfo);
+
+        if (routeParamValues) {
+          routeParamValues.shift(); // Remove the initial match
+
+          routeParamValues
+            .map(decodeURIComponent)
+            .forEach(function(val, indx) {
+              if (route.paramNames && route.paramNames.length > indx) {
+                req.routeParams[route.paramNames[indx]] = val;
+              } else if (val !== undefined) {
+                req.routeParams.splat = req.routeParams.splat || [];
+                req.routeParams.splat.push(val);
+              }
+            });
+        }        
+      }
+    }
+  });
 }