Merge pull request #3677 from airqo-platform/en-validation

Refactor and enhance validateOptionalObjectId middleware with comprehensive unit tests

src/device-registry/bin/server.js

@@ -13,7 +13,7 @@ connectToMongoDB();
 const morgan = require("morgan");
 const compression = require("compression");
 const helmet = require("helmet");
-const { HttpError } = require("@utils/errors");
+const { HttpError, BadRequestError } = require("@utils/errors");
 const isDev = process.env.NODE_ENV === "development";
 const isProd = process.env.NODE_ENV === "production";
 const options = { mongooseConnection: mongoose.connection };
@@ -83,6 +83,12 @@ app.use(function(err, req, res, next) {
         message: err.message,
         errors: err.errors,
       });
+    } else if (err instanceof BadRequestError) {
+      return res.status(err.statusCode).json({
+        success: false,
+        message: err.message,
+        errors: err.errors,
+      });
     } else if (err instanceof SyntaxError) {
       res.status(400).json({
         success: false,
@@ -136,9 +142,9 @@ app.use(function(err, req, res, next) {
       logger.error(`Internal Server Error --- ${stringify(err)}`);
       logObject("Internal Server Error", err);
       logger.error(`Stack Trace: ${err.stack}`);
-      res.status(err.status || 500).json({
+      res.status(err.statusCode || err.status || 500).json({
         success: false,
-        message: "Internal Server Error - app entry",
+        message: err.message || "Internal Server Error",
         errors: { message: err.message },
       });
     }

src/device-registry/middleware/test/ut_validateOptionalObjectId.js

@@ -0,0 +1,100 @@
+require("module-alias/register");
+const { expect } = require("chai");
+const sinon = require("sinon");
+const mongoose = require("mongoose");
+const { BadRequestError } = require("@utils/errors");
+const validateOptionalObjectId = require("@middleware/validateOptionalObjectId");
+
+describe("validateOptionalObjectId", () => {
+  let req, res, next;
+
+  beforeEach(() => {
+    req = {
+      query: {},
+    };
+    res = {};
+    next = sinon.spy();
+  });
+
+  afterEach(() => {
+    sinon.restore();
+  });
+
+  it("should call next() if the field is not present in the query", () => {
+    const middleware = validateOptionalObjectId("testField");
+    middleware(req, res, next);
+    expect(next.calledOnce).to.be.true;
+    expect(next.args[0]).to.be.empty;
+  });
+
+  it("should validate a single valid ObjectId", () => {
+    const validObjectId = new mongoose.Types.ObjectId().toString();
+    req.query.testField = validObjectId;
+    const middleware = validateOptionalObjectId("testField");
+    middleware(req, res, next);
+    expect(next.calledOnce).to.be.true;
+    expect(next.args[0]).to.be.empty;
+  });
+
+  it("should validate multiple valid ObjectIds", () => {
+    const validObjectIds = [
+      new mongoose.Types.ObjectId().toString(),
+      new mongoose.Types.ObjectId().toString(),
+    ];
+    req.query.testField = validObjectIds.join(",");
+    const middleware = validateOptionalObjectId("testField");
+    middleware(req, res, next);
+    expect(next.calledOnce).to.be.true;
+    expect(next.args[0]).to.be.empty;
+  });
+
+  it("should throw BadRequestError for a single invalid ObjectId", () => {
+    req.query.testField = "invalidObjectId";
+    const middleware = validateOptionalObjectId("testField");
+    expect(() => middleware(req, res, next)).to.throw(BadRequestError);
+    expect(next.called).to.be.false;
+    try {
+      middleware(req, res, next);
+    } catch (error) {
+      expect(error).to.be.instanceOf(BadRequestError);
+      expect(error.message).to.equal("Validation failed for testField");
+      expect(error.errors).to.deep.equal([
+        "Invalid testField format: invalidObjectId",
+      ]);
+    }
+  });
+
+  it("should throw BadRequestError for multiple ObjectIds with some invalid", () => {
+    const validObjectId = new mongoose.Types.ObjectId().toString();
+    req.query.testField = `${validObjectId},invalidObjectId`;
+    const middleware = validateOptionalObjectId("testField");
+    expect(() => middleware(req, res, next)).to.throw(BadRequestError);
+    expect(next.called).to.be.false;
+    try {
+      middleware(req, res, next);
+    } catch (error) {
+      expect(error).to.be.instanceOf(BadRequestError);
+      expect(error.message).to.equal("Validation failed for testField");
+      expect(error.errors).to.deep.equal([
+        "Invalid testField format: invalidObjectId",
+      ]);
+    }
+  });
+
+  it("should handle an array of ObjectIds in the query", () => {
+    const validObjectId = new mongoose.Types.ObjectId().toString();
+    req.query.testField = [validObjectId, "invalidObjectId"];
+    const middleware = validateOptionalObjectId("testField");
+    expect(() => middleware(req, res, next)).to.throw(BadRequestError);
+    expect(next.called).to.be.false;
+    try {
+      middleware(req, res, next);
+    } catch (error) {
+      expect(error).to.be.instanceOf(BadRequestError);
+      expect(error.message).to.equal("Validation failed for testField");
+      expect(error.errors).to.deep.equal([
+        "Invalid testField format: invalidObjectId",
+      ]);
+    }
+  });
+});

src/device-registry/middleware/validateOptionalObjectId.js

@@ -0,0 +1,32 @@
+const { isValidObjectId } = require("mongoose");
+const { BadRequestError } = require("@utils/errors");
+
+const validateOptionalObjectId = (field) => {
+  return (req, res, next) => {
+    if (req.query[field]) {
+      let values;
+      if (Array.isArray(req.query[field])) {
+        values = req.query[field];
+      } else {
+        values = req.query[field].toString().split(",");
+      }
+
+      const errors = [];
+      for (const value of values) {
+        if (!isValidObjectId(value)) {
+          errors.push(`Invalid ${field} format: ${value}`);
+        }
+      }
+
+      if (errors.length > 0) {
+        throw new BadRequestError({
+          message: `Validation failed for ${field}`,
+          errors: errors,
+        });
+      }
+    }
+    next();
+  };
+};
+
+module.exports = validateOptionalObjectId;

src/device-registry/routes/v2/readings.js

@@ -4,41 +4,8 @@ const eventController = require("@controllers/create-event");
 const constants = require("@config/constants");
 const mongoose = require("mongoose");
 const ObjectId = mongoose.Types.ObjectId;
-const { logElement, logText, logObject } = require("@utils/log");
-const NetworkModel = require("@models/Network");
-const {
-  check,
-  oneOf,
-  query,
-  body,
-  param,
-  validationResult,
-} = require("express-validator");
-
-const decimalPlaces = require("decimal-places");
-const numeral = require("numeral");
-
-// Define a custom function to check if a value is a valid ObjectId
-const isValidObjectId = (value) => {
-  return mongoose.Types.ObjectId.isValid(value);
-};
-
-const addCategoryQueryParam = (req, res, next) => {
-  req.query.path = "public";
-  next();
-};
-
-const validNetworks = async () => {
-  const networks = await NetworkModel("airqo").distinct("name");
-  return networks.map((network) => network.toLowerCase());
-};
-
-const validateNetwork = async (value) => {
-  const networks = await validNetworks();
-  if (!networks.includes(value.toLowerCase())) {
-    throw new Error("Invalid network");
-  }
-};
+const { oneOf, query, validationResult } = require("express-validator");
+const validateOptionalObjectId = require("@middleware/validateOptionalObjectId");
 
 const validatePagination = (req, res, next) => {
   let limit = parseInt(req.query.limit, 10);
@@ -57,40 +24,6 @@ const validatePagination = (req, res, next) => {
   next();
 };
 
-// Custom validation function to check if values are valid MongoDB ObjectIds
-const isValidObjectIds = (value) => {
-  const ids = value.split(",");
-  return ids.every((id) => /^[0-9a-fA-F]{24}$/.test(id)); // Check if each ID is a valid ObjectId
-};
-
-// Middleware for validation
-const validateObjectId = (paramName) => {
-  return [
-    query(paramName)
-      .custom(isValidObjectIds)
-      .withMessage(`Invalid ${paramName}`),
-  ];
-};
-
-const validateOptionalObjectId = (field) => {
-  return (req, res, next) => {
-    if (req.query[field]) {
-      let values;
-      if (Array.isArray(req.query[field])) {
-        values = req.query[field];
-      } else {
-        values = req.query[field].toString().split(",");
-      }
-      for (const value of values) {
-        if (!isValidObjectId(value)) {
-          throw new Error(`Invalid ${field} format: ${value}`);
-        }
-      }
-    }
-    next();
-  };
-};
-
 const headers = (req, res, next) => {
   res.setHeader("Access-Control-Allow-Origin", "*");
   res.header(
@@ -244,7 +177,7 @@ router.get(
       query("device_id")
         .optional()
         .notEmpty()
-        .withMessage("the provided device_id cannot be empty IF provided")
+        .withMessage("device_id cannot be empty IF provided")
         .trim(),
       query("lat_long")
         .optional()
@@ -277,8 +210,7 @@ router.get(
       query("site_id")
         .optional()
         .notEmpty()
-        .withMessage("the provided site_id cannot be empty IF provided")
-        .trim(),
+        .withMessage("site_id cannot be empty IF provided"),
       query("primary")
         .optional()
         .notEmpty()

src/device-registry/utils/errors.js

@@ -12,6 +12,15 @@ class HttpError extends Error {
   }
 }
 
+class BadRequestError extends Error {
+  constructor({ message, errors }) {
+    super(message);
+    this.name = "BadRequestError";
+    this.statusCode = 400;
+    this.errors = errors;
+  }
+}
+
 const convertErrorArrayToObject = (arrays) => {
   const initialValue = {};
   return arrays.reduce((obj, item) => {
@@ -42,5 +51,6 @@ const extractErrorsFromRequest = (req) => {
 
 module.exports = {
   HttpError,
+  BadRequestError,
   extractErrorsFromRequest,
 };