Originally posted at michaelzanggl.com. Subscribe to my newsletter to never miss out on new content.
Today we will take a look at controllers and how they can grow into big junks of code, and how we can avoid this.
For the example we have an API endpoint that creates an article.
This is the route:
// routes.js
Route.group(() => {
Route.post("store", "ArticleController.store").middleware("auth");
}).prefix("article");
At first our controller looks more or less fine.
'use strict'
const Article = use('App/Models/Article')
class ArticleController {
async store({ params, auth, request }) {
const article = await Article.create({
title: request.input('title'),
description: request.input('description'),
user_id: auth.user.id,
})
return response.json({
article: article.toJSON()
})
}
}
But now we get additional requirements. Articles have tags and we have to save them in the ArticleTag table. We quickly implement it, but then realized that we also have to make sure that the title and description are actually filled. So we implement validation. Since it is not much, we simply add all the code to the controller. A couple days later though we receive the requirements that we should send a mail to all followers and also need a password verification. Again, not much, let's just add it to the controller!
Now that's quite a lot. And it's especially a lot when considering that everything is in this one controller method. It would look something like this:
'use strict'
/** @type {import('@adonisjs/framework/src/Hash')} */
const Hash = use('Hash')
const Article = use('App/Models/Article')
const ArticleTag = use('App/Models/ArticleTag')
class ArticleController {
async store({ params, auth, request }) {
// validation rules
const rules = {
title: 'required',
description: 'required',
}
const validation = await validate(request.all(), rules)
if (validation.fails()) {
return response.status(400).json({
message: validation.messages()[0].messsage,
})
}
// verify password
if (!(await Hash.verify(request.input('password'), auth.user.password))) {
return response.status(400).json({
message: 'The entered password is not correct',
})
}
// actual work
const article = await Article.create({
title: request.input('title'),
description: request.input('description'),
user_id: auth.user.id,
})
const tags = JSON.parse(request.input('tags'))
const articleTags = tags.map(tagId => {
article_id: article.id,
tag_id: tagId
})
await ArticleTag.createMany(articleTags)
// some afterwork
await this.sendMailToFollowers(article)
return response.json({
article: article.toJSON()
})
}
sendMailToFollowers(article) {
// some big private method
}
}
That's a lot of code! And the controller just has one method so far. Let's see how Adonis helps us to clean things up.
Validation
The first piece of code we want to take a look at is the validation part.
// validation rules
const rules = {
title: "required",
description: "required"
};
const validation = await validate(request.all(), rules);
if (validation.fails()) {
return response.status(400).json({
message: validation.messages()[0].messsage
});
}
This can be extracted 100% by putting the validation into its own validator. Adonis offers the following command to create such a validator.
adonis make:validator StoreArticle
and this is the implementation:
"use strict";
class StoreArticle {
get rules() {
return {
title: "required",
description: "required"
};
}
async fails(errorMessages) {
return this.ctx.response.status(400).json({
message: errorMessages[0].message
});
}
}
module.exports = StoreArticle;
Now we just have to add the validator to our existing route.
// routes.js
Route.group(() => {
Route.post("store", "ArticleController.store")
.middleware("auth")
.validator("StoreArticle");
}).prefix("article");
and since the fails part is always the same, once we have more than one validator, we could create a BaseValidator class and always extend from it.
Okay, that's one down, but there is still a lot of room for improvement.
Next let's check out the password verifcation. Chances are that we need this in more than one place. It would also be nice to have this seperated, so it can easily be removed if specs change. Keeping it in the controller simply feels out of place here. A good place to put it is inside a middleware.
Middlewares
The part we want to get rid of is this here.
if (!(await Hash.verify(request.input("password"), auth.user.password))) {
return response.status(400).json({
message: "The entered password is not correct"
});
}
So let's create a middleware for it.
adonis make:middleware VerifyPassword
And here is the implementation.
"use strict";
/** @type {import('@adonisjs/framework/src/Hash')} */
const Hash = use("Hash");
class VerifyPassword {
async handle({ request, auth, response }, next, properties) {
if (!(await Hash.verify(request.input("password"), auth.user.password))) {
return response.status(400).json({
message: "The entered password is not correct"
});
}
await next();
}
}
module.exports = VerifyPassword;
Next we add it to the named middlewares in start/kernel.js.
const namedMiddleware = {
// ...
verifyPassword: "App/Middleware/VerifyPassword"
};
All that is left now is to add the middleware to the route.
// routes.js
Route.group(() => {
Route.post("store", "ArticleController.store")
.middleware(["auth", "verifyPassword"])
.validator("StoreArticle");
}).prefix("article");
This could have also been solved by extending the validator and adding another validation rule to the StoreArticle.js validator.
Events
If operations don't need to be executed immediately, we can execute them asynchroniously using Events. This is perfect for things like sending mails.
This is exactly the case with this line of code here.
await this.sendMailToFollowers(article)
First let's create an event listener:
adonis make:listener Article
This will create App/Listeners/Article.js and here is its implementation:
"use strict";
const Article = (exports = module.exports = {});
const Mail = use("Mail");
Article.registered = async article => {
console.log('mail implementation')
};
Back in ArticleController.js let's add this line to the top:
const Event = use("Event");
All that is left now is to switch out
await this.sendMailToFollowers(article)
with this line:
Event.fire("new::article", article)
Our controller boiled down to just this.
'use strict'
const Event = use("Event");
const Article = use('App/Models/Article')
const ArticleTag = use('App/Models/ArticleTag')
class ArticleController {
async store({ params, auth, request }) {
const article = await Article.create({
title: "request.input('title'),"
description: "request.input('description'),"
user_id: auth.user.id,
})
const tags = JSON.parse(request.input('tags'))
const articleTags = tags.map(tagId => {
article_id: article.id,
tag_id: tagId
})
await ArticleTag.createMany(articleTags)
Event.fire("new::article", article)
return response.json({
article: article.toJSON()
})
}
}
But we can clean this up even more. Right now, we can only create an article when going through this controller. If we need to be able to create articles in other places, e.g. commands, or simply want to make our code more testable, we can move the business logic to a service.
Services
Let's check out the implementation, there is no command for creating services.
// app/Services/ArticleService.js
'use strict'
const Article = use('App/Models/Article')
const ArticleTag = use('App/Models/ArticleTag')
class ArticleService {
async store({ title, description, tags }, user) {
const article = await Article.create({
title,
description,
user_id: user.id,
})
const articleTags = tags.map(tagId => {
article_id: article.id,
tag_id: tagId
})
await ArticleTag.createMany(articleTags)
return article
}
}
module.exports = ArticleService
and our controller now is simply
'use strict'
const Event = use('Event')
const ArticleService = use('App/Services/ArticleService')
class ArticleController {
constructor() {
this.articleService = new ArticleService
}
async store({ params, auth, request }) {
const article = await this.articleService.store(request.all(), auth.user)
Event.fire("new::article", article);
return response.json({
article: article.toJSON()
})
}
}
No custom actions
So far we only looked at refactoring one method inside a controller. You can still end up with pretty big controllers.
If your controller ends up having too many methods, you can start splitting methods into more controllers. How? By keeping the controller cruddy. You can create a resourceful controller in Adonis with the following command:
adonis make:controller YourController --resource
This way the controller has the seven default crud actions. If you need a custom action, make it cruddy and put it in a new controller. What do I mean by this exactly and how can you achieve this?
Well, there's actually a whole talk about this which you can find here.
Conclusion
Great! Each part is now in its appropriate place, is easily testable and reusable. The controller simply calls each part. We were even able to decouple the context (request, auth and response) from the business logic, making the code less coupled to the framework.
Please note that none of these refactorings are strictly necessary. It is okay to get a little messy in the controller at first as you may not have a clear picture of the whole problem you are trying to solve.
But we are not protected from everything yet!
Take a look at the following controller to see what we will refactor in a future article!
const Post = use('App/Models/Post')
class PostsController {
async search({ response, request }) {
const query = Post.query()
if (request.input('category_id')) {
query.where('category_id', request.input('category_id'))
}
let keyword = request.input('keyword')
if (keyword) {
keyword = `%${decodeURIComponent(keyword)}%`
query
.where('title', 'like', keyword)
.orWhere('description', 'like', keyword)
}
const tags = request.input('tags')
if (tags) {
query.whereIn('tags', tags)
}
const posts = await query.fetch()
return response.json({ posts: posts.toJSON() })
}
}
If this article helped you, I have a lot more tips on simplifying writing software here.
Top comments (4)
What about module.exports = new ArticleService() instead of calling it on constructor?
Really great article. Thanks! :)
Then you would always get the same instance of ArticleService. In this case it would maybe not be a problem, but it would be inconsistent with the rest of the application I'd assume. But you are definitely right that it was not the best decision to simply new it up in the constructor. A better way would be to use Adonis' automatic dependency injection so it gets newed up automatically and is easy to fake.
Interesting article. When will the Post Controller refactoring come out ?
Thanks, I've just starting writing the module for it.