1
\$\begingroup\$

I this the function createQuery is fully functionally, but I would like a review on it. I'm pretty sure this can be shorten and most important (improve performance). Any ideas?

const express = require("express");
const router = express.Router();
const CompanyPart = require("../models/parts");

router.post("/search", (req, res) => {
  function createQuery(bodyFieldName, mongodbFieldName) {
    if (bodyFieldName !== undefined && bodyFieldName !== "") {
      const trimmedString = bodyFieldName.trim();
      if (
        trimmedString.charAt(0) !== "*" &&
        trimmedString.charAt(trimmedString.length - 1) !== "*"
      ) {
        // screw
        return (query[mongodbFieldName] = new RegExp(
          "^" + trimmedString + "$",
          "i"
        ));
      } else if (
        trimmedString.charAt(0) === "*" &&
        trimmedString.charAt(trimmedString.length - 1) === "*"
      ) {
        // *screw*
        return (query[mongodbFieldName] = new RegExp(
          trimmedString.replace(/[*]/g, ""),
          "i"
        ));
      } else if (
        trimmedString.charAt(0) !== "*" &&
        trimmedString.charAt(trimmedString.length - 1) === "*"
      ) {
        // screw*
        return (query[mongodbFieldName] = new RegExp(
          "^" + trimmedString.replace(/[*]/g, ""),
          "i"
        ));
      } else if (
        trimmedString.charAt(0) === "*" &&
        trimmedString.charAt(trimmedString.length - 1) !== "*"
      ) {
        // *screw
        return (query[mongodbFieldName] = new RegExp(
          trimmedString.replace(/[*]/g, "") + "$",
          "i"
        ));
      }
    }
  }

  console.log("req.body", req.body);
  let query = {};

  createQuery(req.body.partNumber, "itemNumber");
  createQuery(req.body.partName, "partName");
  createQuery(req.body.customsTariff, "customsTariff");
  console.log("query", query);

  CompanyPart.find(query)
    .sort({
      itemNumber: 1
    })
    .exec((err, parts) => {
      if (err) {
        res.status(500).json(err);
        return;
      }
      res.status(200).json({ parts });
    });
});

module.exports = router;
\$\endgroup\$
1
\$\begingroup\$

First, using new RegExp on user input is a bit like running eval on user input (though not nearly as bad - usually). If it can be avoided, it should be. In this case, if I pass .*? as the partName, the regex will be /^.*?$/i which will match every entry in the database, or I could pass ( which would result in the function throwing an error. This can be mitigated by escaping all special characters, but still isn't a good idea if there is another way to do this. (I haven't used MongoDB, but it appears the $search key might be useful - MongoDB Docs.


If the built in $search parameter doesn't work for you, then the function should escape all special characters in the search string before creating a RegExp with it.

One way to do this is the following:

function createRegex(userInput) {
  return new RegExp(
    // Escape all special characters except *
    "^" + userInput.replace(/([.+?^=!:${}()|\[\]\/\\])/g, "\\$1")
      // Allow the use of * as a wildcard like % in SQL.
      .replace(/\*/g, ".*") + "$",
    'i'
  );
}

['test', '*test*', '*test', 'test*', '[tes{2}t.'].forEach(test => {
  console.log(test, '=>', createRegex(test))
});

While this may be slightly slower than the current function, it is much safer and can greatly simplify the createQuery function. It also allows createQuery to cover the edge case where * appears in the middle of a string.

createQuery can now be implemented in only a few lines.

function createQuery(userInput, dbField) {
  if (userInput === undefined) {
    return;
  }
  // Trim the input before checking as otherwise searching for a
  // single space could cause problems.
  userInput = userInput.trim();
  if (userInput !== "") {
    query[dbField] = createRegex(userInput);
  }
}

Alternatively, this could be shortened (at the cost of some readability) with

function createQuery(userInput, dbField) {
  if (userInput && userInput = userInput.trim()) {
    query[dbField] = createRegex(userInput);
  }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.