Clean Node - Part 1

le 05/03/2018 par Simon Renoult
Tags: Software Engineering

I have been working with Node.js for almost 6 years now (started back in 2012 with 0.6.10). During these years, co-workers have been asking me the same question over and over again: "What does your application look like?". It is now time to answer this question (or at least try to) !

Coming from Java, Python or Ruby, the question might sound irrelevant since frameworks such as Spring, Django or Rails and patterns like layered architectures or clean architectures offered answers some time ago. However, if you have worked with Node.js, you might be able to relate. Indeed, the Node.js ecosystem has thrived these past years thanks to NPM, its package manager (among other things, of course). And even though NPM did a really good job at making small pieces of code available to all, the community has never really communicated on how should these pieces be orchestrated together. As a proof, take a look at https://github.com/sindresorhus/awesome-nodejs that gathers useful Node.js resources. This repository has 20k stars on GitHub and is an awesome list of cool and well-maintained libraries. Yet, I challenge you to find a single link discussing how can these pieces be assembled together in order to build a consistent and maintainable product.

Funnily enough, answers to this question exist and have been discussed for years: SOLID principles, Clean Architecture, Hexagonal Architecture, Clean Code, eXtreme Programming, TDD, BDD, DDD, etc. All these techniques and principles help us design our code better. What I think is definitely lacking though is the application of these to the Node.js realm.

This article is first of a three-part series discussing this exact topic and suggesting potential solutions. Each article will be using a test-first approach, either unit or end-to-end:

  • This first article will set a common ground: quick and dirty version with end to end tests and discussing common mistakes, general opinions and misconceptions
  • Second article will refactor the “quick and dirty” version with unit tests and a layered architecture
  • Third article will refactor the code using the Clean Architecture and unit tests

Ideas and implementations are 100% open to discussions. Comment, issues and PRs are very welcome: https://github.com/simonrenoult/nodejs-application-architecture

Gears and features

In order to get started, we need a few tools and materials we can play with. To keep things mainstream and easily applicable to your context, we'll stick to widespread technologies:

We now need a use-case. Again, nothing fancy but some business rules to be realistic:

  • Shopping API with products, orders and bills.
  • Products
    • Can be created and listed
    • Have an identifier, a name, a price and a weight
    • Products can be sorted by name, price or weight
  • Orders
    • Can be created and listed
    • Have a status, a product list, a shipment amount, a total amount and a weight
    • Orders status can be one of pending, paid or canceled
    • Are offered 5% discount when the price exceeds 1000€
    • Shipment costs 25€ for every 50kg (50€ for 100kg, 75€ for 150kg, etc.)
  • Bills
    • Can be listed
    • Have an amount and a creation date
    • Are automatically generated when an order status is set to paid

Down the rabbit hole…

Quick and dirty

Most Node.js tools and frameworks embrace the first rule of the UNIX philosophy: “do one thing and do it well”. What has been missing however is the translation of the cooperation rules. Which means that in Node.js, the look of your application architecture mostly boils down to your software engineering experience.

To address this issue, we’ll start with a “quick and dirty” version in order to reveal recurrent design flaws Node.js developers can make. Please keep in mind that this code base is a combination of several common mistakes and might look silly to most of you.

Let’s code!

Application scaffold

We’ll start with a server using Express (note: we separated the server starting instructions in bin/start to ease testing):

const express = require("express");
const app = express();

// code will go here

module.exports = app;

We now add the body parser and the database connection which requires some refactoring in order to use async/await and a configuration file:

const express = require("express");
const bodyParser = require("body-parser");
const Sequelize = require("sequelize");

const env = process.env.NODE_ENV || "development";
const conf = require(path.resolve(__dirname, "conf", env));

const sequelize = new Sequelize(
  conf.db.database,
  conf.db.username,
  conf.db.password,
  conf.db.sequelize
);

module.exports = async () => {
  const app = express();
  await sequelize.sync();
  app.use(bodyParser.json());

  // code will go here

  return app;
}

We’re now set to start the real work!

A bit of methodology

First I will start by choosing which set of features (that I call “epic”) I want my API to offer: product creation. In order to achieve this epic, I need to fulfil the following requirements:

Success cases:

  • API returns HTTP code 201 when it succeeds
  • API returns the resource location in the header Location

Error cases:

  • API returns HTTP code 400 when product data is invalid
  • API returns all the keys in error

We want our product to be fully functionally tested. Why? Because writing specifications as automated tests is future-proof: it prevents code updates and refactoring to break any production behavior. This implies the translation of our specifications (listed above) into end-to-end tests.

This practice is a derivative from ATDD where we start with product behavior specifications written as tests and only then implement these behaviors. This practice is also great to create discussions with product owners and business experts in order to tackle unexpected cases that might arise while discussing features. I say “derivative” because in this context there is no business expert.

The first feature

I like to start with error cases in order to get rid of the noise they create once we start thinking about the actual feature. Hence writing a test to check the HTTP code returned when product payload is invalid:

describe("POST /products", () => {
  describe("When product data is invalid", () => {
    it("returns 400", async () => {
      const { statusCode } = await queryApi("POST", "/products", { body: {} });
      expect(statusCode).to.equal(400);
    });
  });
});

The matching implementation:

// API route to create a product
app.post("/products", async (req, res) => {
  if (Object.keys(req.body).length === 0) {
    return res.status(400).send();
  }
});

The second feature

We now test that error keys can be found in the response body:

describe("POST /products", () => {
  describe("When product data is invalid", () => {
    it("returns the keys in error", async () => {
      const { body, statusCode } = await queryApi("POST", "/products", { body: {} });
      const contextList = body.data.map(item => item.context.key);
      expect(statusCode).to.equal(400);
      expect(contextList).to.contain("name", "price", "weight");
    });
  });
});

The matching implementation:

// Schemas and models
const productSchema = Joi.object().keys({
  name: Joi.required(),
  price: Joi.required(),
  weight: Joi.required()
});

// …

// API route to create a product
app.post("/products", async (req, res) => {
  const { error } = Joi.validate(req.body, productSchema, { abortEarly: false });
  if (error) {
    const errorMessage = error.details.map(({ message, context }) => { message, context });
    return res.status(400).send({ data: errorMessage });
  }
});

The third feature

We can now check the success cases, starting with the server HTTP code response:

describe("POST /products", () => {
  describe("When product data is valid", () => {
    it("returns 201", async () => {
      const product = { name: "tshirt", price: 20, weight: 0.1 };
      const { statusCode } = await queryApi("POST", "/products", {body: data});
      expect(statusCode).to.equal(201);
    });
  });
});

The matching implementation:

// API route to create a product
app.post("/products", async (req, res) => {
  const { error } = Joi.validate(req.body, productSchema, { abortEarly: false });

  if (error) {
    const errorMessage = error.details.map(({ message, context }) =>
      Object.assign({ message, context })
    );
    return res.status(400).send({ data: errorMessage });
  }

  const product = await Product.create(req.body);
  res.status(201).send();
});

The fourth feature

We will now test that the API returns the resource location:

describe("POST /products", () => {
  describe("When product data is valid", () => {
    it("returns the product location", async () => {
      const data = { name: "tshirt", price: 20, weight: 0.1 };
      const { headers } = await queryApi("POST", "/products", {body: data});
      expect(headers.location).to.match(/products\/.+/);
    });
  });
});

The matching implementation:


// API route to create a product
app.post("/products", async (req, res) => {
  const { error } = Joi.validate(req.body, productSchema, { abortEarly: false });
  
  if (error) {
    const errorMessage = error.details.map(({ message, context }) =>
      Object.assign({ message, context })
    );
    return res.status(400).send({ data: errorMessage });
  }

  const product = await Product.create(req.body);
  res.set("Location", `/products/${product.id}`);
  res.status(201).send();
});

A pattern emerges...

Can you see where this practice leads us? We start adding blobs and blobs of code within the same anonymous function responsible of the product creation. And why would we do otherwise? It makes perfect sense from our functional test suite perspective. We have done what we were told. We could try to refactor but there is no incentive to go a different way we did and we have no idea which way to go.

What's cool?

This code base has a few qualities which will help us refactor it:

  • Relatively short: 200 lines
  • Super low global complexity
  • End to end tests
  • Linter to maintain code style
  • Variables are correctly named
  • Magic strings and number are extracted

What's wrong?

Where did we end up? We have added a few hundred lines of code with tests in total good faith. However, as the code grew, we kind of felt like something was going wrong, but what is it?

Well, it mostly boils down to two major design flaws:

  • Tight coupling
  • Multiple responsibility

See for yourself, the `index.js` file does almost everything:

  • Server initialization
  • Database connection
  • Environment support
  • Logging
  • Route declaration
  • HTTP deserialization
  • Configuration logic
  • Business logic
  • Database bindings
  • HTTP serialization

What's more, business and infrastructure logic depend on each other. For instance, in order to create an order, which is a real-life business use-case, we must extract information from the request body (infrastructure), then apply some format validation in order to work on consistent data (infrastructure), then calculate prices and discounts (business) and finally send the appropriate HTTP code (infrastructure).

But why would we like our code to be different? What changes would that make?

Dealing with the consequences

Software development is the encounter of business and crafting skills. In order to achieve this marriage with the highest outcome possible: as many features in the shortest time possible, the current code is kind of a pain to deal with for many reasons we list below.

Heavy cognitive load

One has to keep many things in mind to comprehend how the code works at the very top level of our application. How does the API achieve an order creation? A product listing? This should be visible in one glance. As you can see, it is not.

Also, the code lacks expressiveness. We do things but we never give these things a name. Instead, we have added comments that, while bringing meanings and explanations, also bring a lot of noise.

These are all technical details that hide both developer and business intentions.

Example:

How does one know how to create an order? Well, looking at the code we can see this:

// HTTP request payload validation
// abortEarly is false in order to retrieve all the errors at once
const { error } = Joi.validate(req.body, orderSchema, {
  abortEarly: false
});

if (error) {
  // Create the HTTP response error list
  const errorMessage = error.details.map(({ message, context }) =>
    Object.assign({ message, context })
  );

  return res.status(400).send({ data: errorMessage });
}

// Fetch the list of products based on the products provided in the order
const productList = await Product.findAll({
  where: {
    id: { [Op.in]: req.body.product_list.map(id => parseInt(id, 0)) }
  }
});

if (productList.length === 0) {
  return res.status(400).send({
    data: [
      { message: "Unknown products", context: { key: "product_list" } }
    ]
  });
}

const productListData = productList.map(product => product.toJSON());

// Compute the total weight order
const orderTotalWeight = productListData
  .map(p => p.weight)
  .reduce((prev, cur) => prev + cur, 0);

// Compute the total price amount
const orderProductListPrice = productListData
  .map(p => p.price)
  .reduce((prev, cur) => prev + cur, 0);

// Compute the shipment price amount
const SHIPMENT_PRICE_STEP = 25;
const SHIPMENT_WEIGHT_STEP = 10;
const orderShipmentPrice =
  SHIPMENT_PRICE_STEP * Math.round(orderTotalWeight / SHIPMENT_WEIGHT_STEP);

// Compute the discount
let totalAmount = orderProductListPrice + orderShipmentPrice;
if (totalAmount > 1000) {
  totalAmount = totalAmount * 0.95;
}

const orderData = Object.assign(
  {
    total_amount: totalAmount,
    shipment_amount: orderShipmentPrice,
    total_weight: orderTotalWeight
  },
  { product_list: req.body.product_list }
);

const order = await Order.create(orderData);
res.set("Location", `/orders/${order.id}`);

res.status(201).send();

This anonymous function is 50 lines long and does many things. However, what creating an order really boils down to is (from both technical and business perspectives):

  • Validate what comes in the API
  • Send HTTP error when something goes wrong
  • Retrieve the product list from the order
  • Calculate the shipment price, total weight and amount
  • Save the order in the database
  • Return the appropriate HTTP code

An appropriate code design should reflect these steps and hide the implementation details using higher levels of abstraction. By doing so, we are going to make tradeoffs, because that’s what design is: decrease the local complexity and increase the global one.

Side-effects everywhere

I started writing this application with functional tests first then implementing the requirements expressed in these tests. This is a great start in order to preserve API features across refactoring and code updates but it does not address the inner application design.

As revealed by our current implementation, we designed the code with a single application layer, hitting all the infrastructure layers: HTTP and database.

Doing a single (functional) test loop does not enforce any unit test driven development initiative. Such an approach would have helped us make the application design emerge off the unit test suite.

No code reuse

Our code base does not scale and is error-prone.

The piece of code below contains the error mapping logic and is repeated twice (lines 85 and 126). Duplicating this code each time we want the appropriate error message means that modifications have to be made at several locations in our code, implying more occasions to forget things or make mistakes.

const errorMessage = error.details.map(({ message, context }) =>
  Object.assign({ message, context })
);

Functional tests take time to run

Our functional test suite hits both network and database layers, this means four (de)serialization steps :

  • HTTP to application
  • Application to database
  • Database to application
  • Application to HTTP

Plus the database response delay.

If we consider that making a read operation on our database takes 50ms and a write operation takes 100ms, the test suite will take 22s to execute (as shown in the commit a877fc), which is awfully long for a feedback loop. And that's just on our 200 lines application.

No way to test at the unit level

What if the business comes to me and ask to change the discount logic? Currently, I have to update my functional tests which, as mentioned before, are already in charge of ensuring many steps of the application logic.

Why should I bother with infrastructure details when all I want to ensure is the business logic my product owner asked me to implement? How do I get quick feedback on whether I broke an already existing rule?

Well, at the moment, we literally can’t: our design does not allow it. Here is the code we would like to test:

if (totalAmount > 1000) {
  totalAmount = totalAmount * 0.95;
}

And this code is located between instructions doing completely different things! Above these are a few lines calculating the total amount and the shipment weight and prices, which are a completely unrelated set of business rules:

const SHIPMENT_PRICE_STEP = 25;
const SHIPMENT_WEIGHT_STEP = 10;

const orderShipmentPrice =
  SHIPMENT_PRICE_STEP * Math.round(orderTotalWeight / SHIPMENT_WEIGHT_STEP);

let totalAmount = orderProductListPrice + orderShipmentPrice;

And below these lines is the creation of the client response body, which has more to do with infrastructure details than business logic:

const orderData = Object.assign(
  {
    total_amount: totalAmount,
    shipment_amount: orderShipmentPrice,
    total_weight: orderTotalWeight
  },
  { product_list: req.body.product_list }
);

Generally speaking, we have no way to test specific parts of our application. This leads to inappropriate test practices made of longer feedback loop (because tests will take more time to execute) and a black box approach since our test practice does not allow us to drive our implementation and make it de facto modular.

Misconceptions regarding software quality

Linting is the level 0 of software quality

Linting fills unanswered questions of the language, time consuming questions people love to debate: semicolons? Tabs or spaces? 80 columns? Etc.

Don’t get me wrong though: linting is mandatory. It gets rid of all the unnecessary noise and helps developers focus on what really matters: developing product features while preserving software quality.

Stick to a standard and never worry about it again.

Functional testing has nothing to do with software design quality

Whether it is unit, integration, functional or something else, testing is always a good practice. But different testing practices mean different costs and purposes.

What functional testing ensures is non-regression and feature documentation. What it costs in terms of development time is up to your team experience and choices. What it brings to your software design is nothing.

What's next?

OK, so where does this leave us?

Well, we observed many flaws, most of them related to responsibility, readability, coupling and testing practices. In order to solve these issues, I am going to refactor the current code with two well-known approaches:

  • TDD using unit tests
  • Layered architecture

What I am not saying though, is that these techniques will solve all of our problems. Rather, I bet it will offer an other perspective on how you can write, test and organize your Node.js code base.

Hope you liked it and see you soon!