Moving common logic into middleware

Let's see how we can improve our code further. If you examine our Create User endpoint handler, you may notice that its logic could be applied to all requests. For example, if a request comes in carrying a payload, we expect the value of its Content-Type header to include the string application/json, regardless of which endpoint it is hitting. Therefore, we should pull that piece of logic out into middleware functions to maximize reusability. Specifically, these middleware should perform the following checks:

• If a request uses the method POST, PUT or PATCH, it must carry a non-empty payload.
• If a request contains a non-empty payload, it should have its Content-Type header set. If it doesn't, respond with the 400 Bad Request status code.
• If a request has set its Content-Type header, it must contain the string application/json. If it doesn't, respond with the 415 Unsupported Media Type status code.

Let's translate these criteria into Cucumber/Gherkin specifications. Since these are generic requirements, we should create a new file at spec/cucumber/features/main.feature and define our scenarios there. Have a go at it yourself; once you're done, compare it with the following solution:

Feature: General

  Scenario Outline: POST, PUT and PATCH requests should have non-empty payloads

  All POST, PUT and PATCH requests must have non-zero values for its "Content-Length" header

    When the client creates a <method> request to /users
    And attaches a generic empty payload
    And sends the request
    Then our API should respond with a 400 HTTP status code
    And the payload of the response should be a JSON object
    And contains a message property which says 'Payload should not be empty'

    Examples:
    | method |
    | POST   |
    | PATCH  |
    | PUT    |

  Scenario: Content-Type Header should be set for requests with non-empty payloads

  All requests which has non-zero values for its "Content-Length" header must have its "Content-Type" header set

    When the client creates a POST request to /users
    And attaches a generic non-JSON payload
    But without a "Content-Type" header set
    And sends the request
    Then our API should respond with a 400 HTTP status code
    And the payload of the response should be a JSON object
    And contains a message property which says 'The "Content-Type" header must be set for requests with a non-empty payload'

  Scenario: Content-Type Header should be set to application/json

  All requests which has a "Content-Type" header must set its value to contain "application/json"

    When the client creates a POST request to /users
    And attaches a generic non-JSON payload
    And sends the request
    Then our API should respond with a 415 HTTP status code
    And the payload of the response should be a JSON object
    And contains a message property which says 'The "Content-Type" header must always be "application/json"'

When we run our tests, the step But without a "Content-Type" header set shows up as undefined; so let's implement it. It is as simple as running the unset method on a superagent's request object:

When(/^without a (?:"|')([\w-]+)(?:"|') header set$/, function (headerName) {
  this.request.unset(headerName);
});

Run the tests and see that all steps are now defined but some are failing. Red. Green. Refactor. We're at the red stage, so let's modify our application code so that it'll pass (green). Again, have a go at it yourself, and compare it with our solution here once you're done:

...
function checkEmptyPayload(req, res, next) {
  if (
    ['POST', 'PATCH', 'PUT'].includes(req.method)
    && req.headers['content-length'] === '0'
  ) {
    res.status(400);
    res.set('Content-Type', 'application/json');
    res.json({
      message: 'Payload should not be empty',
    });
  }
  next();
}

function checkContentTypeIsSet(req, res, next) {
  if (
    req.headers['content-length']
    && req.headers['content-length'] !== '0'
    && !req.headers['content-type']
  ) {
    res.status(400);
    res.set('Content-Type', 'application/json');
    res.json({ message: 'The "Content-Type" header must be set for requests with a non-empty payload' });
  }
  next();
}

function checkContentTypeIsJson(req, res, next) {
  if (!req.headers['content-type'].includes('application/json')) {
    res.status(415);
    res.set('Content-Type', 'application/json');
    res.json({ message: 'The "Content-Type" header must always be "application/json"' });
  }
  next();
}

app.use(checkEmptyPayload);
app.use(checkContentTypeIsSet);
app.use(checkContentTypeIsJson);
app.use(bodyParser.json({ limit: 1e6 }));

app.post('/users', (req, res, next) => { next(); });
...

It is important to run our tests again to make sure we didn't break existing functionality. On this occasion, they should all be passing. Therefore, the only thing left to do is to commit this refactoring into our Git repository:

$ git add -A
$ git commit -m "Move common logic into middleware functions"