From 38302203f490d09a481baa14a2f2443fc62da94c Mon Sep 17 00:00:00 2001
From: Baptiste Gelez <baptiste@gelez.xyz>
Date: Fri, 14 Dec 2018 23:16:18 +0100
Subject: [PATCH] Count items in database as much as possible (#344)

* Count items in database as much as possible

* Fix the tests

* Remove two useless queries

* Run pragma directive before each sqlite connection

* Pragma for tests too

* Remove debug messages
---
 plume-models/src/blogs.rs                     |  9 +++
 plume-models/src/comments.rs                  |  6 +-
 plume-models/src/db_conn.rs                   | 14 ++++-
 plume-models/src/lib.rs                       |  4 +-
 plume-models/src/notifications.rs             |  8 +++
 plume-models/src/posts.rs                     | 27 +++++---
 plume-models/src/users.rs                     | 63 ++++++++++---------
 src/main.rs                                   |  6 +-
 src/routes/blogs.rs                           |  6 +-
 src/routes/comments.rs                        |  4 +-
 src/routes/instance.rs                        |  4 +-
 src/routes/notifications.rs                   |  2 +-
 src/routes/posts.rs                           |  4 +-
 src/routes/user.rs                            |  2 +-
 templates/blogs/details.rs.html               |  2 +-
 templates/instance/about.rs.html              |  2 +-
 templates/instance/index.rs.html              |  2 +-
 .../partials/instance_description.rs.html     |  2 +-
 templates/posts/details.rs.html               |  2 +-
 19 files changed, 109 insertions(+), 60 deletions(-)

diff --git a/plume-models/src/blogs.rs b/plume-models/src/blogs.rs
index 55292351..dc6a9c6c 100644
--- a/plume-models/src/blogs.rs
+++ b/plume-models/src/blogs.rs
@@ -83,6 +83,15 @@ impl Blog {
             .expect("Blog::list_authors: author loading error")
     }
 
+    pub fn count_authors(&self, conn: &Connection) -> i64 {
+        use schema::blog_authors;
+        blog_authors::table
+            .filter(blog_authors::blog_id.eq(self.id))
+            .count()
+            .get_result(conn)
+            .expect("Blog::count_authors: count loading error")
+    }
+
     pub fn find_for_author(conn: &Connection, author: &User) -> Vec<Blog> {
         use schema::blog_authors;
         let author_ids = blog_authors::table
diff --git a/plume-models/src/comments.rs b/plume-models/src/comments.rs
index 2a9cbf61..b51dd393 100644
--- a/plume-models/src/comments.rs
+++ b/plume-models/src/comments.rs
@@ -56,16 +56,16 @@ impl Comment {
         Post::get(conn, self.post_id).expect("Comment::get_post: post error")
     }
 
-    pub fn count_local(conn: &Connection) -> usize {
+    pub fn count_local(conn: &Connection) -> i64 {
         use schema::users;
         let local_authors = users::table
             .filter(users::instance_id.eq(Instance::local_id(conn)))
             .select(users::id);
         comments::table
             .filter(comments::author_id.eq_any(local_authors))
-            .load::<Comment>(conn)
+            .count()
+            .get_result(conn)
             .expect("Comment::count_local: loading error")
-            .len() // TODO count in database?
     }
 
     pub fn get_responses(&self, conn: &Connection) -> Vec<Comment> {
diff --git a/plume-models/src/db_conn.rs b/plume-models/src/db_conn.rs
index a17ac33f..9bdd4c9a 100644
--- a/plume-models/src/db_conn.rs
+++ b/plume-models/src/db_conn.rs
@@ -1,4 +1,4 @@
-use diesel::r2d2::{ConnectionManager, Pool, PooledConnection};
+use diesel::{dsl::sql_query, r2d2::{ConnectionManager, CustomizeConnection, Error as ConnError, Pool, PooledConnection}, ConnectionError, RunQueryDsl};
 use rocket::{
     http::Status,
     request::{self, FromRequest},
@@ -38,3 +38,15 @@ impl Deref for DbConn {
         &self.0
     }
 }
+
+// Execute a pragma for every new sqlite connection
+#[derive(Debug)]
+pub struct PragmaForeignKey;
+impl CustomizeConnection<Connection, ConnError> for PragmaForeignKey {
+    #[cfg(feature = "sqlite")] // will default to an empty function for postgres
+    fn on_acquire(&self, conn: &mut Connection) -> Result<(), ConnError> {
+        sql_query("PRAGMA foreign_keys = on;").execute(conn)
+            .map(|_| ())
+            .map_err(|_| ConnError::ConnectionError(ConnectionError::BadConnection(String::from("PRAGMA foreign_keys = on failed"))))
+    }
+}
diff --git a/plume-models/src/lib.rs b/plume-models/src/lib.rs
index aa6a1d3e..b8b49ea4 100644
--- a/plume-models/src/lib.rs
+++ b/plume-models/src/lib.rs
@@ -213,7 +213,7 @@ pub fn ap_url(url: &str) -> String {
 #[cfg(test)]
 #[macro_use]
 mod tests {
-    use diesel::Connection;
+    use diesel::{dsl::sql_query, Connection, RunQueryDsl};
     use Connection as Conn;
     use DATABASE_URL;
 
@@ -238,6 +238,8 @@ mod tests {
         let conn =
             Conn::establish(&*DATABASE_URL.as_str()).expect("Couldn't connect to the database");
         embedded_migrations::run(&conn).expect("Couldn't run migrations");
+        #[cfg(feature = "sqlite")]
+        sql_query("PRAGMA foreign_keys = on;").execute(&conn).expect("PRAGMA foreign_keys fail");
         conn
     }
 }
diff --git a/plume-models/src/notifications.rs b/plume-models/src/notifications.rs
index 1fd3fcb9..146cc6de 100644
--- a/plume-models/src/notifications.rs
+++ b/plume-models/src/notifications.rs
@@ -48,6 +48,14 @@ impl Notification {
             .expect("Notification::find_for_user: notification loading error")
     }
 
+    pub fn count_for_user(conn: &Connection, user: &User) -> i64 {
+        notifications::table
+            .filter(notifications::user_id.eq(user.id))
+            .count()
+            .get_result(conn)
+            .expect("Notification::count_for_user: count loading error")
+    }
+
     pub fn page_for_user(
         conn: &Connection,
         user: &User,
diff --git a/plume-models/src/posts.rs b/plume-models/src/posts.rs
index 18446f53..333ff728 100644
--- a/plume-models/src/posts.rs
+++ b/plume-models/src/posts.rs
@@ -12,7 +12,6 @@ use serde_json;
 
 use blogs::Blog;
 use instance::Instance;
-use likes::Like;
 use medias::Media;
 use mentions::Mention;
 use plume_api::posts::PostEndpoint;
@@ -24,7 +23,6 @@ use plume_common::{
     utils::md_to_html,
 };
 use post_authors::*;
-use reshares::Reshare;
 use safe_string::SafeString;
 use search::Searcher;
 use schema::posts;
@@ -206,7 +204,7 @@ impl Post {
             .expect("Post::count_for_tag: no result error")
     }
 
-    pub fn count_local(conn: &Connection) -> usize {
+    pub fn count_local(conn: &Connection) -> i64 {
         use schema::post_authors;
         use schema::users;
         let local_authors = users::table
@@ -218,9 +216,9 @@ impl Post {
         posts::table
             .filter(posts::id.eq_any(local_posts_id))
             .filter(posts::published.eq(true))
-            .load::<Post>(conn)
+            .count()
+            .get_result(conn)
             .expect("Post::count_local: loading error")
-            .len() // TODO count in database?
     }
 
     pub fn count(conn: &Connection) -> i64 {
@@ -271,6 +269,15 @@ impl Post {
             .expect("Post::get_for_blog:: loading error")
     }
 
+    pub fn count_for_blog(conn: &Connection, blog: &Blog) -> i64 {
+        posts::table
+            .filter(posts::blog_id.eq(blog.id))
+            .filter(posts::published.eq(true))
+            .count()
+            .get_result(conn)
+            .expect("Post::count_for_blog:: count error")
+    }
+
     pub fn blog_page(conn: &Connection, blog: &Blog, (min, max): (i32, i32)) -> Vec<Post> {
         posts::table
             .filter(posts::blog_id.eq(blog.id))
@@ -379,19 +386,21 @@ impl Post {
             .expect("Post::get_blog: no result error")
     }
 
-    pub fn get_likes(&self, conn: &Connection) -> Vec<Like> {
+    pub fn count_likes(&self, conn: &Connection) -> i64 {
         use schema::likes;
         likes::table
             .filter(likes::post_id.eq(self.id))
-            .load::<Like>(conn)
+            .count()
+            .get_result(conn)
             .expect("Post::get_likes: loading error")
     }
 
-    pub fn get_reshares(&self, conn: &Connection) -> Vec<Reshare> {
+    pub fn count_reshares(&self, conn: &Connection) -> i64 {
         use schema::reshares;
         reshares::table
             .filter(reshares::post_id.eq(self.id))
-            .load::<Reshare>(conn)
+            .count()
+            .get_result(conn)
             .expect("Post::get_reshares: loading error")
     }
 
diff --git a/plume-models/src/users.rs b/plume-models/src/users.rs
index 1187ec79..49e80098 100644
--- a/plume-models/src/users.rs
+++ b/plume-models/src/users.rs
@@ -30,16 +30,13 @@ use std::cmp::PartialEq;
 use url::Url;
 use webfinger::*;
 
-use blog_authors::BlogAuthor;
 use blogs::Blog;
 use db_conn::DbConn;
 use follows::Follow;
 use instance::*;
-use likes::Like;
 use medias::Media;
 use post_authors::PostAuthor;
 use posts::Post;
-use reshares::Reshare;
 use safe_string::SafeString;
 use schema::users;
 use search::Searcher;
@@ -111,7 +108,7 @@ impl User {
 
         Blog::find_for_author(conn, self)
             .iter()
-            .filter(|b| b.list_authors(conn).len() <= 1)
+            .filter(|b| b.count_authors(conn) <= 1)
             .for_each(|b| b.delete(conn, searcher));
         // delete the posts if they is the only author
         let all_their_posts_ids: Vec<i32> = post_authors::table
@@ -170,12 +167,12 @@ impl User {
         User::get(conn, self.id).expect("User::update: get error")
     }
 
-    pub fn count_local(conn: &Connection) -> usize {
+    pub fn count_local(conn: &Connection) -> i64 {
         users::table
             .filter(users::instance_id.eq(Instance::local_id(conn)))
-            .load::<User>(conn)
+            .count()
+            .get_result(conn)
             .expect("User::count_local: loading error")
-            .len() // TODO count in database?
     }
 
     pub fn find_local(conn: &Connection, username: &str) -> Option<User> {
@@ -641,6 +638,16 @@ impl User {
             .expect("User::get_followers: loading error")
     }
 
+    pub fn count_followers(&self, conn: &Connection) -> i64 {
+        use schema::follows;
+        let follows = Follow::belonging_to(self).select(follows::follower_id);
+        users::table
+            .filter(users::id.eq_any(follows))
+            .count()
+            .get_result(conn)
+            .expect("User::count_followers: counting error")
+    }
+
     pub fn get_followers_page(&self, conn: &Connection, (min, max): (i32, i32)) -> Vec<User> {
         use schema::follows;
         let follows = Follow::belonging_to(self).select(follows::follower_id);
@@ -663,52 +670,52 @@ impl User {
 
     pub fn is_followed_by(&self, conn: &Connection, other_id: i32) -> bool {
         use schema::follows;
-        !follows::table
+        follows::table
             .filter(follows::follower_id.eq(other_id))
             .filter(follows::following_id.eq(self.id))
-            .load::<Follow>(conn)
-            .expect("User::is_followed_by: loading error")
-            .is_empty() // TODO count in database?
+            .count()
+            .get_result::<i64>(conn)
+            .expect("User::is_followed_by: loading error") > 0
     }
 
     pub fn is_following(&self, conn: &Connection, other_id: i32) -> bool {
         use schema::follows;
-        !follows::table
+        follows::table
             .filter(follows::follower_id.eq(self.id))
             .filter(follows::following_id.eq(other_id))
-            .load::<Follow>(conn)
-            .expect("User::is_following: loading error")
-            .is_empty() // TODO count in database?
+            .count()
+            .get_result::<i64>(conn)
+            .expect("User::is_following: loading error") > 0
     }
 
     pub fn has_liked(&self, conn: &Connection, post: &Post) -> bool {
         use schema::likes;
-        !likes::table
+        likes::table
             .filter(likes::post_id.eq(post.id))
             .filter(likes::user_id.eq(self.id))
-            .load::<Like>(conn)
-            .expect("User::has_liked: loading error")
-            .is_empty() // TODO count in database?
+            .count()
+            .get_result::<i64>(conn)
+            .expect("User::has_liked: loading error") > 0
     }
 
     pub fn has_reshared(&self, conn: &Connection, post: &Post) -> bool {
         use schema::reshares;
-        !reshares::table
+        reshares::table
             .filter(reshares::post_id.eq(post.id))
             .filter(reshares::user_id.eq(self.id))
-            .load::<Reshare>(conn)
-            .expect("User::has_reshared: loading error")
-            .is_empty() // TODO count in database?
+            .count()
+            .get_result::<i64>(conn)
+            .expect("User::has_reshared: loading error") > 0
     }
 
     pub fn is_author_in(&self, conn: &Connection, blog: &Blog) -> bool {
         use schema::blog_authors;
-        !blog_authors::table
+        blog_authors::table
             .filter(blog_authors::author_id.eq(self.id))
             .filter(blog_authors::blog_id.eq(blog.id))
-            .load::<BlogAuthor>(conn)
-            .expect("User::is_author_in: loading error")
-            .is_empty() // TODO count in database?
+            .count()
+            .get_result::<i64>(conn)
+            .expect("User::is_author_in: loading error") > 0
     }
 
     pub fn get_keypair(&self) -> PKey<Private> {
@@ -1173,7 +1180,7 @@ pub(crate) mod tests {
                 last_username = page[0].username.clone();
             }
             assert_eq!(
-                User::get_local_page(conn, (0, User::count_local(conn) as i32 + 10)).len(),
+                User::get_local_page(conn, (0, User::count_local(conn) as i32 + 10)).len() as i64,
                 User::count_local(conn)
             );
 
diff --git a/src/main.rs b/src/main.rs
index 61bdf20f..dd7843df 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -39,7 +39,7 @@ use diesel::r2d2::ConnectionManager;
 use rocket::State;
 use rocket_csrf::CsrfFairingBuilder;
 use plume_models::{DATABASE_URL, Connection,
-    db_conn::DbPool, search::Searcher as UnmanagedSearcher};
+    db_conn::{DbPool, PragmaForeignKey}, search::Searcher as UnmanagedSearcher};
 use scheduled_thread_pool::ScheduledThreadPool;
 use std::process::exit;
 use std::sync::Arc;
@@ -59,7 +59,9 @@ fn init_pool() -> Option<DbPool> {
     dotenv::dotenv().ok();
 
     let manager = ConnectionManager::<Connection>::new(DATABASE_URL.as_str());
-    DbPool::new(manager).ok()
+    DbPool::builder()
+        .connection_customizer(Box::new(PragmaForeignKey))
+        .build(manager).ok()
 }
 
 fn main() {
diff --git a/src/routes/blogs.rs b/src/routes/blogs.rs
index 3cfab251..bb880110 100644
--- a/src/routes/blogs.rs
+++ b/src/routes/blogs.rs
@@ -29,7 +29,7 @@ pub fn details(intl: I18n, name: String, conn: DbConn, user: Option<User>, page:
     let blog = Blog::find_by_fqn(&*conn, &name)
         .ok_or_else(|| render!(errors::not_found(&(&*conn, &intl.catalog, user.clone()))))?;
     let posts = Post::blog_page(&*conn, &blog, page.limits());
-    let articles = Post::get_for_blog(&*conn, &blog); // TODO only count them in DB
+    let articles_count = Post::count_for_blog(&*conn, &blog);
     let authors = &blog.list_authors(&*conn);
 
     Ok(render!(blogs::details(
@@ -37,9 +37,9 @@ pub fn details(intl: I18n, name: String, conn: DbConn, user: Option<User>, page:
         blog.clone(),
         blog.get_fqn(&*conn),
         authors,
-        articles.len(),
+        articles_count,
         page.0,
-        Page::total(articles.len() as i32),
+        Page::total(articles_count as i32),
         user.map(|x| x.is_author_in(&*conn, &blog)).unwrap_or(false),
         posts
     )))
diff --git a/src/routes/comments.rs b/src/routes/comments.rs
index 534964b4..144e6512 100644
--- a/src/routes/comments.rs
+++ b/src/routes/comments.rs
@@ -75,8 +75,8 @@ pub fn create(blog_name: String, slug: String, form: LenientForm<NewCommentForm>
                 Tag::for_post(&*conn, post.id),
                 comments.into_iter().filter(|c| c.in_response_to_id.is_none()).collect::<Vec<Comment>>(),
                 previous,
-                post.get_likes(&*conn).len(),
-                post.get_reshares(&*conn).len(),
+                post.count_likes(&*conn),
+                post.count_reshares(&*conn),
                 user.has_liked(&*conn, &post),
                 user.has_reshared(&*conn, &post),
                 user.is_following(&*conn, post.get_authors(&*conn)[0].id),
diff --git a/src/routes/instance.rs b/src/routes/instance.rs
index 76a19d46..d8b7f69e 100644
--- a/src/routes/instance.rs
+++ b/src/routes/instance.rs
@@ -37,8 +37,8 @@ pub fn index(conn: DbConn, user: Option<User>, intl: I18n) -> Ructe {
             render!(instance::index(
                 &(&*conn, &intl.catalog, user),
                 inst,
-                User::count_local(&*conn) as i32,
-                Post::count_local(&*conn) as i32,
+                User::count_local(&*conn),
+                Post::count_local(&*conn),
                 local,
                 federated,
                 user_feed
diff --git a/src/routes/notifications.rs b/src/routes/notifications.rs
index 6f8ba1d0..cad67d8f 100644
--- a/src/routes/notifications.rs
+++ b/src/routes/notifications.rs
@@ -13,7 +13,7 @@ pub fn notifications(conn: DbConn, user: User, page: Option<Page>, intl: I18n) -
         &(&*conn, &intl.catalog, Some(user.clone())),
         Notification::page_for_user(&*conn, &user, page.limits()),
         page.0,
-        Page::total(Notification::find_for_user(&*conn, &user).len() as i32)
+        Page::total(Notification::count_for_user(&*conn, &user) as i32)
     ))
 }
 
diff --git a/src/routes/posts.rs b/src/routes/posts.rs
index 4e530768..70181872 100644
--- a/src/routes/posts.rs
+++ b/src/routes/posts.rs
@@ -66,8 +66,8 @@ pub fn details(blog: String, slug: String, conn: DbConn, user: Option<User>, res
             Tag::for_post(&*conn, post.id),
             comments.into_iter().filter(|c| c.in_response_to_id.is_none()).collect::<Vec<Comment>>(),
             previous,
-            post.get_likes(&*conn).len(),
-            post.get_reshares(&*conn).len(),
+            post.count_likes(&*conn),
+            post.count_reshares(&*conn),
             user.clone().map(|u| u.has_liked(&*conn, &post)).unwrap_or(false),
             user.clone().map(|u| u.has_reshared(&*conn, &post)).unwrap_or(false),
             user.map(|u| u.is_following(&*conn, post.get_authors(&*conn)[0].id)).unwrap_or(false),
diff --git a/src/routes/user.rs b/src/routes/user.rs
index f10ceda9..70d68bbe 100644
--- a/src/routes/user.rs
+++ b/src/routes/user.rs
@@ -168,7 +168,7 @@ pub fn follow_auth(name: String, i18n: I18n) -> Flash<Redirect> {
 pub fn followers(name: String, conn: DbConn, account: Option<User>, page: Option<Page>, intl: I18n) -> Result<Ructe, Ructe> {
     let page = page.unwrap_or_default();
     let user = User::find_by_fqn(&*conn, &name).ok_or_else(|| render!(errors::not_found(&(&*conn, &intl.catalog, account.clone()))))?;
-    let followers_count = user.get_followers(&*conn).len(); // TODO: count in DB
+    let followers_count = user.count_followers(&*conn);
 
     Ok(render!(users::followers(
         &(&*conn, &intl.catalog, account.clone()),
diff --git a/templates/blogs/details.rs.html b/templates/blogs/details.rs.html
index b4c11bab..79814d52 100644
--- a/templates/blogs/details.rs.html
+++ b/templates/blogs/details.rs.html
@@ -5,7 +5,7 @@
 @use template_utils::*;
 @use routes::*;
 
-@(ctx: BaseContext, blog: Blog, fqn: String, authors: &Vec<User>, total_articles: usize, page: i32, n_pages: i32, is_author: bool, posts: Vec<Post>)
+@(ctx: BaseContext, blog: Blog, fqn: String, authors: &Vec<User>, total_articles: i64, page: i32, n_pages: i32, is_author: bool, posts: Vec<Post>)
 
 @:base(ctx, blog.title.as_ref(), {}, {
     <a href="@uri!(blogs::details: name = &fqn, page = _)">@blog.title</a>
diff --git a/templates/instance/about.rs.html b/templates/instance/about.rs.html
index 9e38a1a3..2abe6f70 100644
--- a/templates/instance/about.rs.html
+++ b/templates/instance/about.rs.html
@@ -3,7 +3,7 @@
 @use template_utils::*;
 @use routes::*;
 
-@(ctx: BaseContext, instance: Instance, admin: User, n_users: usize, n_articles: usize, n_instances: i64)
+@(ctx: BaseContext, instance: Instance, admin: User, n_users: i64, n_articles: i64, n_instances: i64)
 
 @:base(ctx, i18n!(ctx.1, "About {0}"; instance.name.clone()).as_str(), {}, {}, {
     <h1>@i18n!(ctx.1, "About {0}"; instance.name)</h1>
diff --git a/templates/instance/index.rs.html b/templates/instance/index.rs.html
index e1d4cb66..c8fa3ea6 100644
--- a/templates/instance/index.rs.html
+++ b/templates/instance/index.rs.html
@@ -4,7 +4,7 @@
 @use plume_models::posts::Post;
 @use routes::*;
 
-@(ctx: BaseContext, instance: Instance, n_users: i32, n_articles: i32, local: Vec<Post>, federated: Vec<Post>, user_feed: Option<Vec<Post>>)
+@(ctx: BaseContext, instance: Instance, n_users: i64, n_articles: i64, local: Vec<Post>, federated: Vec<Post>, user_feed: Option<Vec<Post>>)
 
 @:base(ctx, instance.name.clone().as_ref(), {}, {}, {
   <h1>@i18n!(ctx.1, "Welcome on {}"; instance.name.as_str())</h1>
diff --git a/templates/partials/instance_description.rs.html b/templates/partials/instance_description.rs.html
index 6411dca0..67f08818 100644
--- a/templates/partials/instance_description.rs.html
+++ b/templates/partials/instance_description.rs.html
@@ -2,7 +2,7 @@
 @use plume_models::instance::Instance;
 @use routes::*;
 
-@(ctx: BaseContext, instance: Instance, n_users: i32, n_articles: i32)
+@(ctx: BaseContext, instance: Instance, n_users: i64, n_articles: i64)
 
 <section class="spaced">
     <div class="cards">
diff --git a/templates/posts/details.rs.html b/templates/posts/details.rs.html
index 72d2ee46..52ba3c38 100644
--- a/templates/posts/details.rs.html
+++ b/templates/posts/details.rs.html
@@ -9,7 +9,7 @@
 @use routes::comments::NewCommentForm;
 @use routes::*;
 
-@(ctx: BaseContext, article: Post, blog: Blog, comment_form: &NewCommentForm, comment_errors: ValidationErrors, tags: Vec<Tag>, comments: Vec<Comment>, previous_comment: Option<Comment>, n_likes: usize, n_reshares: usize, has_liked: bool, has_reshared: bool, is_following: bool, author: User)
+@(ctx: BaseContext, article: Post, blog: Blog, comment_form: &NewCommentForm, comment_errors: ValidationErrors, tags: Vec<Tag>, comments: Vec<Comment>, previous_comment: Option<Comment>, n_likes: i64, n_reshares: i64, has_liked: bool, has_reshared: bool, is_following: bool, author: User)
 
 @:base(ctx, &article.title.clone(), {
     <meta property="og:title" content="@article.title"/>