-
Notifications
You must be signed in to change notification settings - Fork 398
Description
Article.toJson
individually gets author. We should get them as a join instead. Same for tags. Otherwise we get 3 queries per row (article author, tag and tag count) / or 5 if logged in (+1 for user likes article, +1 for user follows author), which is insane.
Doing http://localhost:3000/api/articles?limit=2&offset=0 while logged off leads to the following reordered/organized list of requests according to DEBUG='*:sql:*'
:
article find and count all, this is fine
sequelize:sql:sqlite Executing (default): SELECT count(`Article`.`id`) AS `count` FROM `Article` AS `Article` LEFT OUTER JOIN `User` AS `author` ON `Article`.`authorId` = `author`.`id`; +5s
sequelize:sql:sqlite Executing (default): SELECT `Article`.`id`, `Article`.`slug`, `Article`.`title`, `Article`.`description`, `Article`.`body`, `Article`.`createdAt`, `Article`.`updatedAt`, `Article`.`authorId`, `author`.`id` AS `author.id`, `author`.`username` AS `author.username`, `author`.`email` AS `author.email`, `author`.`bio` AS `author.bio`, `author`.`image` AS `author.image`, `author`.`hash` AS `author.hash`, `author`.`salt` AS `author.salt`, `author`.`createdAt` AS `author.createdAt`, `author`.`updatedAt` AS `author.updatedAt` FROM `Article` AS `Article` LEFT OUTER JOIN `User` AS `author` ON `Article`.`authorId` = `author`.`id` ORDER BY `Article`.`createdAt` DESC LIMIT 0, 2; +0ms
post 499
author: sequelize:sql:sqlite Executing (default): SELECT `id`, `username`, `email`, `bio`, `image`, `hash`, `salt`, `createdAt`, `updatedAt` FROM `User` AS `User` WHERE `User`.`id` = 9; +0ms
tags: sequelize:sql:sqlite Executing (default): SELECT `Tag`.`id`, `Tag`.`name`, `Tag`.`createdAt`, `Tag`.`updatedAt`, `ArticleTag`.`createdAt` AS `ArticleTag.createdAt`, `ArticleTag`.`updatedAt` AS `ArticleTag.updatedAt`, `ArticleTag`.`articleId` AS `ArticleTag.articleId`, `ArticleTag`.`tagId` AS `ArticleTag.tagId` FROM `Tag` AS `Tag` INNER JOIN `ArticleTag` AS `ArticleTag` ON `Tag`.`id` = `ArticleTag`.`tagId` AND `ArticleTag`.`articleId` = 499; +0ms
favoriteCount: sequelize:sql:sqlite Executing (default): SELECT COUNT(`User`.`id`) AS `count` FROM `User` AS `User` INNER JOIN `UserFavoriteArticle` AS `UserFavoriteArticle` ON `User`.`id` = `UserFavoriteArticle`.`userId` AND `UserFavoriteArticle`.`articleId` = 499; +1ms
post 500: same 3 queries
Of those, we could remove all the users and tags queries with joins!
The users JOIN was already in place, we just weren't checking for it properly. Fixed with:
diff --git a/models/article.js b/models/article.js
index 1f76561..4a2ccd0 100644
--- a/models/article.js
+++ b/models/article.js
@@ -111,8 +111,8 @@ module.exports = (sequelize) => {
}
Article.prototype.toJson = async function(user) {
- let authorPromise;
- if (this.authorPromise === undefined) {
+ let authorPromise
+ if (!this.author) {
authorPromise = this.getAuthor()
} else {
authorPromise = new Promise(resolve => {resolve(this.author)})
Had tried to fix it previously, but failed because this.authorPromise === undefined
was always true!
The authorPromise.then(author => author.toProfileJSONFor(user)),
would however make one extra query to see if the current user is following that user or not if were logged in. We would need to also join by following to fix that.