Correct PATCH and PUT behavior / Add property select capability on populate alterable

lib/handlers.js

@@ -33,120 +33,183 @@ exports.schema = function(req, res, next) {
   next();
 };
 
-exports.get = function(req, res, next) {
+exports.get = async function(req, res, next) {
   if (req.params.id && !mongoose.Types.ObjectId.isValid(req.params.id)) {
     exports.respond(res, 404, exports.objectNotFound());
     next();
   } else {
-    req.quer.exec(function(err, list) {
-      if (err) {
-        exports.respond(res, 500, err);
-      } else if (req.params.id) {
+    try {
+      // Execute the built query (thenable) without using exec()
+      const list = await req.quer;
+      if (req.params.id) {
         exports.respondOrErr(res, 404, !list && exports.objectNotFound(), 200, (list && _.isArray(list)) ? list[0] : list);
+        next();
       } else {
-        exports.respondOrErr(res, 500, err, 200, list);
+        try {
+          // Use model-level countDocuments with the same filter to avoid reusing the query instance
+          const filter = (typeof req.quer.getFilter === 'function') ? req.quer.getFilter() : req.quer._conditions || {};
+          const total = await req.quer.model.countDocuments(filter);
+          res.set('X-Total-Count', total);
+          exports.respond(res, 200, list);
+          next();
+        } catch (err) {
+          exports.respond(res, 500, err);
+          next();
+        }
       }
+    } catch (err) {
+      exports.respond(res, 500, err);
       next();
-    });
+    }
   }
 };
 
-exports.getDetail = function(req, res, next) {
-  req.quer.exec(function(err, one) {
-    exports.respondOrErr(res, 500, err, 200, one);
+exports.getDetail = async function(req, res, next) {
+  try {
+    const one = await req.quer;
+    exports.respond(res, 200, one);
     next();
-  });
+  } catch (err) {
+    exports.respond(res, 500, err);
+    next();
+  }
 };
 
 /**
  * Generates a handler that returns the object at @param pathName
  * where pathName is the path to an objectId field
  */
 exports.getPath = function(pathName) {
-  return function(req, res, next) {
+  return async function(req, res, next) {
     req.quer = req.quer.populate(pathName);
-    req.quer.exec(function(err, one) {
+    try {
+      const one = await req.quer;
+      exports.respond(res, 200, (one && one.get(pathName)) || {});
+      next();
+    } catch (err) {
       var errStatus = ((err && err.status) ? err.status : 500);
-      exports.respondOrErr(res, errStatus, err, 200, (one && one.get(pathName)) || {});
+      exports.respond(res, errStatus, err);
       next();
-    });
+    }
   };
 };
 
-exports.post = function(req, res, next) {
+exports.post = async function(req, res, next) {
   var obj = new this(req.body);
-  obj.save(function(err) {
-    exports.respondOrErr(res, 400, err, 201, obj);
+  try {
+    await obj.save();
+    exports.respond(res, 201, obj);
     next();
-  });
+  } catch (err) {
+    exports.respond(res, 400, err);
+    next();
+  }
 };
 
-exports.put = function(req, res, next) {
+exports.put = async function(req, res, next) {
+  // Remove immutable ObjectId from update attributes to prevent request failure
+  if (req.body._id && req.body._id === req.params.id) {
+    delete req.body._id;
+  }
+
+  // Update in 1 atomic operation on the database if not specified otherwise
+  try {
+    const newObj = await req.quer.findOneAndReplace({'_id': req.params.id}, req.body, { omitUndefined: true, ...this.update_options});
+    if (!newObj) {
+      exports.respond(res, 404, exports.objectNotFound());
+    } else {
+      exports.respond(res, 200, newObj);
+    }
+    next();
+  } catch (err) {
+    exports.respond(res, 500, err);
+    next();
+  }
+};
+
+exports.patch = async function(req, res, next) {
   // Remove immutable ObjectId from update attributes to prevent request failure
   if (req.body._id && req.body._id === req.params.id) {
     delete req.body._id;
   }
 
   // Update in 1 atomic operation on the database if not specified otherwise
   if (this.shouldUseAtomicUpdate) {
-    req.quer.findOneAndUpdate({}, req.body, this.update_options, function(err, newObj) {
-      if (err) {
-        exports.respond(res, 500, err);
-      } else if (!newObj) {
+    try {
+      const newObj = await req.quer.findOneAndUpdate({}, req.body, this.update_options);
+      if (!newObj) {
         exports.respond(res, 404, exports.objectNotFound());
       } else {
         exports.respond(res, 200, newObj);
       }
       next();
-    });
+    } catch (err) {
+      exports.respond(res, 500, err);
+      next();
+    }
   } else {
     // Preform the update in two operations allowing mongoose to fire its schema update hook
-    req.quer.findOne({"_id": req.params.id}, function(err, docToUpdate) {
-      if (err) {
-        exports.respond(res, 500, err);
-      }
+    try {
+      const docToUpdate = await req.quer.findOne({"_id": req.params.id});
       var objNotFound = !docToUpdate && exports.objectNotFound();
       if (objNotFound) {
         exports.respond(res, 404, objNotFound);
         return next();
       }
 
       docToUpdate.set(req.body);
-      docToUpdate.save(function (err, obj) {
-        exports.respondOrErr(res, 400, err, 200, obj);
+      try {
+        const obj = await docToUpdate.save();
+        exports.respond(res, 200, obj);
+        next();
+      } catch (err) {
+        exports.respond(res, 400, err);
         next();
-      });
-    });
+      }
+    } catch (err) {
+      exports.respond(res, 500, err);
+      next();
+    }
   }
 };
 
-exports.delete = function(req, res, next) {
+exports.delete = async function(req, res, next) {
   // Delete in 1 atomic operation on the database if not specified otherwise
   if (this.shouldUseAtomicUpdate) {
-    req.quer.findOneAndRemove({}, this.delete_options, function(err, obj) {
-      if (err) {
-        exports.respond(res, 500, err);
+    try {
+      const obj = await req.quer.findOneAndRemove({}, this.delete_options);
+      if (!obj) {
+        exports.respond(res, 404, exports.objectNotFound());
+      } else {
+        exports.respond(res, 204, {});
       }
-      exports.respondOrErr(res, 404, !obj && exports.objectNotFound(), 204, {});
       next();
-    });
+    } catch (err) {
+      exports.respond(res, 500, err);
+      next();
+    }
   } else {
     // Preform the remove in two steps allowing mongoose to fire its schema update hook
-    req.quer.findOne({"_id": req.params.id}, function(err, docToRemove) {
-      if (err) {
-        exports.respond(res, 500, err);
-      }
+    try {
+      const docToRemove = await req.quer.findOne({"_id": req.params.id});
       var objNotFound = !docToRemove && exports.objectNotFound();
       if (objNotFound) {
         exports.respond(res, 404, objNotFound);
         return next();
       }
 
-      docToRemove.remove(function (err, obj) {
-        exports.respondOrErr(res, 400, err, 204, {});
+      try {
+        await docToRemove.remove();
+        exports.respond(res, 204, {});
         next();
-      });
-    });
+      } catch (err) {
+        exports.respond(res, 400, err);
+        next();
+      }
+    } catch (err) {
+      exports.respond(res, 500, err);
+      next();
+    }
   }
 };
 

lib/model.js

@@ -5,13 +5,14 @@ var mongoose = require('mongoose'),
 
 exports = module.exports = model;
 
-var methods = ['get', 'post', 'put', 'delete'], // All HTTP methods, PATCH not currently supported
-  endpoints = ['get', 'post', 'put', 'delete', 'getDetail'],
+var methods = ['get', 'post', 'put', 'patch', 'delete'], // All HTTP methods, PATCH not currently supported
+  endpoints = ['get', 'post', 'put', 'patch', 'delete', 'getDetail'],
   defaultroutes = ['schema'],
   lookup = {
     'get': 'index',
     'getDetail': 'show',
     'put': 'updated',
+    'patch': 'patched',
     'post': 'created',
     'delete': 'deleted'
   },
@@ -23,6 +24,7 @@ var methods = ['get', 'post', 'put', 'delete'], // All HTTP methods, PATCH not c
       'skip': query('skip'),
       'offset': query('offset'),
       'select': query('select'),
+      'lean': query('lean'),
       'sort': query('sort'),
     }, {
       'equals': query('equals'),
@@ -31,6 +33,7 @@ var methods = ['get', 'post', 'put', 'delete'], // All HTTP methods, PATCH not c
       'lt': query('lt'),
       'lte': query('lte'),
       'ne': query('ne'),
+      'exists': query('exists'),
       'regex': function(val, query) {
         var regParts = val.match(/^\/(.*?)\/([gim]*)$/);
         if (regParts) {
@@ -233,13 +236,16 @@ Model.registerRoutes = function(app, prefix, path, routeObj) {
             [handlers.last]
           );
       /**
-       * TODO(baugarten): Add an enum type-thing to specify detail route, detail optional or list
-       * aka prettify this
+       * Express 5 compatible routes: no anchors in param regex, no optional params with ?.
+       * For GET that supports both list and detail, register two separate routes.
+       * ID validation is done via router.param() in Model.register.
        */
       if (route.detail) {
-        app[key](prefix + '/:id([0-9a-fA-F]{0,24})' + path , handlerlist);
+        app[key](prefix + '/:id' + path , handlerlist);
       } else if (detailGet) {
-        app[key](prefix + '/:id([0-9a-fA-F]{0,24}$)?', handlerlist);
+        // Support both list (GET /resource) and detail (GET /resource/:id) on separate routes
+        app[key](prefix, handlerlist);
+        app[key](prefix + '/:id', handlerlist);
       } else {
         app[key](prefix + path, handlerlist);
       }
@@ -259,6 +265,16 @@ Model.registerRoutes = function(app, prefix, path, routeObj) {
 Model.register = function(app, url) {
   this.addDefaultRoutes();
   app.getDetail = app.get;
+
+  // Express 5 compatible: validate :id param for 24-char hex MongoDB ObjectID
+  // This replaces the regex previously embedded in the route path
+  app.param('id', function(req, res, next, id) {
+    if (!/^[0-9a-fA-F]{24}$/.test(id)) {
+      return res.status(400).json({ error: 'Invalid id format. Expected 24-character hex string.' });
+    }
+    next();
+  });
+
   this.registerRoutes(app, url, '', this.routes);
 };
 
@@ -448,6 +464,16 @@ function coerceData(filter_func, data) {
     return false;
   } else if (filter_func === 'limit' || filter_func === 'skip') {
     return parseInt(data);
+  } else if (filter_func === 'populate') {
+    if (typeof(data) === 'string'){
+      splitedData = data.split(',')
+      return (splitedData.length > 1) ? {path: splitedData[0], select: splitedData[1]} : data
+    } else if (Array.isArray(data)){
+      return data.map(data => {
+        splitedData = data.split(',')
+        return (splitedData.length > 1) ? {path: splitedData[0], select: splitedData[1]} : data
+      })
+    }
   }
   return data;
 };
@@ -473,7 +499,19 @@ function filterable(props, subfilters) {
       if (key in props) return true;
       var field = key.split('__');
       var filter_func = field[1] || 'equals';
-      return field[0] in quer.model.schema.paths && filter_func in subfilters;
+
+      var prop = field[0];
+
+      if (prop.split('.').length) {
+        var path = prop.split('.');
+        var path = quer.model.schema.paths[path[0]];
+
+        if (path && path.instance === 'Array') {
+            prop = prop.split('.')[0];
+        }
+      }
+
+      return prop in quer.model.schema.paths && filter_func in subfilters;
     }
   }
 }

package.json

@@ -1,9 +1,10 @@
 {
   "name": "node-restful",
   "description": "A library for making REST API's in node.js with express",
-  "version": "0.2.6",
+  "version": "0.2.8",
   "peerDependencies": {
-    "mongoose": "~4"
+    "mongoose": "~4 || >=5",
+    "express": "^4.12.0 || ^5.0.0"
   },
   "devDependencies": {
     "express": "4.12.4",