From 2d4099577fe8d9fef2317f1a15808507834d90f7 Mon Sep 17 00:00:00 2001 From: Dessalines Date: Thu, 3 Dec 2020 19:47:58 -0500 Subject: [PATCH] Removing old user_view. --- lemmy_api/src/community.rs | 12 +- lemmy_api/src/site.rs | 22 +-- lemmy_api/src/user.rs | 65 +++++--- lemmy_apub/src/fetcher.rs | 4 +- lemmy_db/src/community.rs | 4 +- lemmy_db/src/lib.rs | 1 - lemmy_db/src/user_view.rs | 279 -------------------------------- lemmy_db/src/views/user_view.rs | 140 +++++++++++++++- lemmy_structs/src/community.rs | 4 +- lemmy_structs/src/site.rs | 9 +- lemmy_structs/src/user.rs | 9 +- 11 files changed, 216 insertions(+), 333 deletions(-) delete mode 100644 lemmy_db/src/user_view.rs diff --git a/lemmy_api/src/community.rs b/lemmy_api/src/community.rs index 762420202..8024e1e27 100644 --- a/lemmy_api/src/community.rs +++ b/lemmy_api/src/community.rs @@ -19,7 +19,7 @@ use lemmy_db::{ naive_now, post::Post, site::*, - user_view::*, + views::user_view::UserViewSafe, Bannable, Crud, Followable, @@ -640,7 +640,7 @@ impl Perform for BanFromCommunity { let user_id = data.user_id; let user_view = blocking(context.pool(), move |conn| { - UserView::get_user_secure(conn, user_id) + UserViewSafe::read(conn, user_id) }) .await??; @@ -748,17 +748,19 @@ impl Perform for TransferCommunity { }) .await??; - let mut admins = blocking(context.pool(), move |conn| UserView::admins(conn)).await??; + let mut admins = blocking(context.pool(), move |conn| UserViewSafe::admins(conn)).await??; let creator_index = admins .iter() - .position(|r| r.id == site_creator_id) + .position(|r| r.user.id == site_creator_id) .context(location_info!())?; let creator_user = admins.remove(creator_index); admins.insert(0, creator_user); // Make sure user is the creator, or an admin - if user.id != read_community.creator_id && !admins.iter().map(|a| a.id).any(|x| x == user.id) { + if user.id != read_community.creator_id + && !admins.iter().map(|a| a.user.id).any(|x| x == user.id) + { return Err(APIError::err("not_an_admin").into()); } diff --git a/lemmy_api/src/site.rs b/lemmy_api/src/site.rs index c62224170..a4e9cfd56 100644 --- a/lemmy_api/src/site.rs +++ b/lemmy_api/src/site.rs @@ -20,8 +20,10 @@ use lemmy_db::{ naive_now, post_view::*, site::*, - user_view::*, - views::site_view::SiteView, + views::{ + site_view::SiteView, + user_view::{UserQueryBuilder, UserViewSafe}, + }, Crud, SearchType, SortType, @@ -281,20 +283,20 @@ impl Perform for GetSite { None }; - let mut admins = blocking(context.pool(), move |conn| UserView::admins(conn)).await??; + let mut admins = blocking(context.pool(), move |conn| UserViewSafe::admins(conn)).await??; // Make sure the site creator is the top admin if let Some(site_view) = site_view.to_owned() { let site_creator_id = site_view.creator.id; // TODO investigate why this is sometimes coming back null // Maybe user_.admin isn't being set to true? - if let Some(creator_index) = admins.iter().position(|r| r.id == site_creator_id) { + if let Some(creator_index) = admins.iter().position(|r| r.user.id == site_creator_id) { let creator_user = admins.remove(creator_index); admins.insert(0, creator_user); } } - let banned = blocking(context.pool(), move |conn| UserView::banned(conn)).await??; + let banned = blocking(context.pool(), move |conn| UserViewSafe::banned(conn)).await??; let online = context .chat_server() @@ -535,15 +537,15 @@ impl Perform for TransferSite { let site_view = blocking(context.pool(), move |conn| SiteView::read(conn)).await??; - let mut admins = blocking(context.pool(), move |conn| UserView::admins(conn)).await??; + let mut admins = blocking(context.pool(), move |conn| UserViewSafe::admins(conn)).await??; let creator_index = admins .iter() - .position(|r| r.id == site_view.creator.id) + .position(|r| r.user.id == site_view.creator.id) .context(location_info!())?; let creator_user = admins.remove(creator_index); admins.insert(0, creator_user); - let banned = blocking(context.pool(), move |conn| UserView::banned(conn)).await??; + let banned = blocking(context.pool(), move |conn| UserViewSafe::banned(conn)).await??; let counts = blocking(context.pool(), move |conn| SiteAggregates::read(conn)).await??; @@ -594,8 +596,8 @@ impl Perform for SaveSiteConfig { let user = get_user_from_jwt(&data.auth, context.pool()).await?; // Only let admins read this - let admins = blocking(context.pool(), move |conn| UserView::admins(conn)).await??; - let admin_ids: Vec = admins.into_iter().map(|m| m.id).collect(); + let admins = blocking(context.pool(), move |conn| UserViewSafe::admins(conn)).await??; + let admin_ids: Vec = admins.into_iter().map(|m| m.user.id).collect(); if !admin_ids.contains(&user.id) { return Err(APIError::err("not_an_admin").into()); diff --git a/lemmy_api/src/user.rs b/lemmy_api/src/user.rs index 693bd6d8c..1f10b4e5b 100644 --- a/lemmy_api/src/user.rs +++ b/lemmy_api/src/user.rs @@ -33,8 +33,10 @@ use lemmy_db::{ user::*, user_mention::*, user_mention_view::*, - user_view::*, - views::site_view::SiteView, + views::{ + site_view::SiteView, + user_view::{UserViewDangerous, UserViewSafe}, + }, Crud, Followable, Joinable, @@ -153,7 +155,7 @@ impl Perform for Register { // Make sure there are no admins let any_admins = blocking(context.pool(), move |conn| { - UserView::admins(conn).map(|a| a.is_empty()) + UserViewSafe::admins(conn).map(|a| a.is_empty()) }) .await??; if data.admin && !any_admins { @@ -490,23 +492,40 @@ impl Perform for GetUserDetails { }; let user_id = user.map(|u| u.id); - let user_fun = move |conn: &'_ _| { - match user_id { - // if there's a logged in user and it's the same id as the user whose details are being - // requested we need to use get_user_dangerous so it returns their email or other sensitive - // data hidden when viewing users other than yourself - Some(auth_user_id) => { - if user_details_id == auth_user_id { - UserView::get_user_dangerous(conn, auth_user_id) - } else { - UserView::get_user_secure(conn, user_details_id) - } - } - None => UserView::get_user_secure(conn, user_details_id), - } - }; - let user_view = blocking(context.pool(), user_fun).await??; + let (user_view, user_dangerous) = if let Some(auth_user_id) = user_id { + if user_details_id == auth_user_id { + ( + None, + Some( + blocking(context.pool(), move |conn| { + UserViewDangerous::read(conn, auth_user_id) + }) + .await??, + ), + ) + } else { + ( + Some( + blocking(context.pool(), move |conn| { + UserViewSafe::read(conn, user_details_id) + }) + .await??, + ), + None, + ) + } + } else { + ( + Some( + blocking(context.pool(), move |conn| { + UserViewSafe::read(conn, user_details_id) + }) + .await??, + ), + None, + ) + }; let page = data.page; let limit = data.limit; @@ -555,7 +574,9 @@ impl Perform for GetUserDetails { // Return the jwt Ok(GetUserDetailsResponse { + // TODO need to figure out dangerous user view here user: user_view, + user_dangerous, follows, moderates, comments, @@ -600,10 +621,10 @@ impl Perform for AddAdmin { }) .await??; - let mut admins = blocking(context.pool(), move |conn| UserView::admins(conn)).await??; + let mut admins = blocking(context.pool(), move |conn| UserViewSafe::admins(conn)).await??; let creator_index = admins .iter() - .position(|r| r.id == site_creator_id) + .position(|r| r.user.id == site_creator_id) .context(location_info!())?; let creator_user = admins.remove(creator_index); admins.insert(0, creator_user); @@ -681,7 +702,7 @@ impl Perform for BanUser { let user_id = data.user_id; let user_view = blocking(context.pool(), move |conn| { - UserView::get_user_secure(conn, user_id) + UserViewSafe::read(conn, user_id) }) .await??; diff --git a/lemmy_apub/src/fetcher.rs b/lemmy_apub/src/fetcher.rs index ec44bce17..fc1857035 100644 --- a/lemmy_apub/src/fetcher.rs +++ b/lemmy_apub/src/fetcher.rs @@ -21,7 +21,7 @@ use lemmy_db::{ post::{Post, PostForm}, post_view::PostView, user::{UserForm, User_}, - user_view::UserView, + views::user_view::UserViewSafe, Crud, Joinable, SearchType, @@ -161,7 +161,7 @@ pub async fn search_by_apub_id( response.users = vec![ blocking(context.pool(), move |conn| { - UserView::get_user_secure(conn, user.id) + UserViewSafe::read(conn, user.id) }) .await??, ]; diff --git a/lemmy_db/src/community.rs b/lemmy_db/src/community.rs index 5f76d5143..845b386c2 100644 --- a/lemmy_db/src/community.rs +++ b/lemmy_db/src/community.rs @@ -144,14 +144,14 @@ impl Community { } fn community_mods_and_admins(conn: &PgConnection, community_id: i32) -> Result, Error> { - use crate::{community_view::CommunityModeratorView, user_view::UserView}; + use crate::{community_view::CommunityModeratorView, views::user_view::UserViewSafe}; let mut mods_and_admins: Vec = Vec::new(); mods_and_admins.append( &mut CommunityModeratorView::for_community(conn, community_id) .map(|v| v.into_iter().map(|m| m.user_id).collect())?, ); mods_and_admins - .append(&mut UserView::admins(conn).map(|v| v.into_iter().map(|a| a.id).collect())?); + .append(&mut UserViewSafe::admins(conn).map(|v| v.into_iter().map(|a| a.user.id).collect())?); Ok(mods_and_admins) } diff --git a/lemmy_db/src/lib.rs b/lemmy_db/src/lib.rs index a4600ac4e..61a2120d1 100644 --- a/lemmy_db/src/lib.rs +++ b/lemmy_db/src/lib.rs @@ -32,7 +32,6 @@ pub mod site; pub mod user; pub mod user_mention; pub mod user_mention_view; -pub mod user_view; pub mod views; pub type DbPool = diesel::r2d2::Pool>; diff --git a/lemmy_db/src/user_view.rs b/lemmy_db/src/user_view.rs deleted file mode 100644 index bf85280ac..000000000 --- a/lemmy_db/src/user_view.rs +++ /dev/null @@ -1,279 +0,0 @@ -use super::user_view::user_fast::BoxedQuery; -use crate::{fuzzy_search, limit_and_offset, MaybeOptional, SortType}; -use diesel::{dsl::*, pg::Pg, result::Error, *}; -use serde::Serialize; - -table! { - user_view (id) { - id -> Int4, - actor_id -> Text, - name -> Varchar, - preferred_username -> Nullable, - avatar -> Nullable, - banner -> Nullable, - email -> Nullable, - matrix_user_id -> Nullable, - bio -> Nullable, - local -> Bool, - admin -> Bool, - banned -> Bool, - show_avatars -> Bool, - send_notifications_to_email -> Bool, - published -> Timestamp, - number_of_posts -> BigInt, - post_score -> BigInt, - number_of_comments -> BigInt, - comment_score -> BigInt, - } -} - -table! { - user_fast (id) { - id -> Int4, - actor_id -> Text, - name -> Varchar, - preferred_username -> Nullable, - avatar -> Nullable, - banner -> Nullable, - email -> Nullable, - matrix_user_id -> Nullable, - bio -> Nullable, - local -> Bool, - admin -> Bool, - banned -> Bool, - show_avatars -> Bool, - send_notifications_to_email -> Bool, - published -> Timestamp, - number_of_posts -> BigInt, - post_score -> BigInt, - number_of_comments -> BigInt, - comment_score -> BigInt, - } -} - -#[derive(Queryable, Identifiable, PartialEq, Debug, Serialize, QueryableByName, Clone)] -#[table_name = "user_fast"] -pub struct UserView { - pub id: i32, - pub actor_id: String, - pub name: String, - pub preferred_username: Option, - pub avatar: Option, - pub banner: Option, - pub email: Option, // TODO this shouldn't be in this view - pub matrix_user_id: Option, - pub bio: Option, - pub local: bool, - pub admin: bool, - pub banned: bool, - pub show_avatars: bool, // TODO this is a setting, probably doesn't need to be here - pub send_notifications_to_email: bool, // TODO also never used - pub published: chrono::NaiveDateTime, - pub number_of_posts: i64, - pub post_score: i64, - pub number_of_comments: i64, - pub comment_score: i64, -} - -pub struct UserQueryBuilder<'a> { - conn: &'a PgConnection, - query: BoxedQuery<'a, Pg>, - sort: &'a SortType, - page: Option, - limit: Option, -} - -impl<'a> UserQueryBuilder<'a> { - pub fn create(conn: &'a PgConnection) -> Self { - use super::user_view::user_fast::dsl::*; - - let query = user_fast.into_boxed(); - - UserQueryBuilder { - conn, - query, - sort: &SortType::Hot, - page: None, - limit: None, - } - } - - pub fn sort(mut self, sort: &'a SortType) -> Self { - self.sort = sort; - self - } - - pub fn search_term>(mut self, search_term: T) -> Self { - use super::user_view::user_fast::dsl::*; - if let Some(search_term) = search_term.get_optional() { - self.query = self.query.filter(name.ilike(fuzzy_search(&search_term))); - } - self - } - - pub fn page>(mut self, page: T) -> Self { - self.page = page.get_optional(); - self - } - - pub fn limit>(mut self, limit: T) -> Self { - self.limit = limit.get_optional(); - self - } - - pub fn list(self) -> Result, Error> { - use super::user_view::user_fast::dsl::*; - use diesel::sql_types::{Nullable, Text}; - - let mut query = self.query; - - query = match self.sort { - SortType::Hot => query - .order_by(comment_score.desc()) - .then_order_by(published.desc()), - SortType::Active => query - .order_by(comment_score.desc()) - .then_order_by(published.desc()), - SortType::New => query.order_by(published.desc()), - SortType::TopAll => query.order_by(comment_score.desc()), - SortType::TopYear => query - .filter(published.gt(now - 1.years())) - .order_by(comment_score.desc()), - SortType::TopMonth => query - .filter(published.gt(now - 1.months())) - .order_by(comment_score.desc()), - SortType::TopWeek => query - .filter(published.gt(now - 1.weeks())) - .order_by(comment_score.desc()), - SortType::TopDay => query - .filter(published.gt(now - 1.days())) - .order_by(comment_score.desc()), - }; - - let (limit, offset) = limit_and_offset(self.page, self.limit); - query = query.limit(limit).offset(offset); - - // The select is necessary here to not get back emails - query = query.select(( - id, - actor_id, - name, - preferred_username, - avatar, - banner, - "".into_sql::>(), - matrix_user_id, - bio, - local, - admin, - banned, - show_avatars, - send_notifications_to_email, - published, - number_of_posts, - post_score, - number_of_comments, - comment_score, - )); - query.load::(self.conn) - } -} - -impl UserView { - pub fn admins(conn: &PgConnection) -> Result, Error> { - use super::user_view::user_fast::dsl::*; - use diesel::sql_types::{Nullable, Text}; - user_fast - // The select is necessary here to not get back emails - .select(( - id, - actor_id, - name, - preferred_username, - avatar, - banner, - "".into_sql::>(), - matrix_user_id, - bio, - local, - admin, - banned, - show_avatars, - send_notifications_to_email, - published, - number_of_posts, - post_score, - number_of_comments, - comment_score, - )) - .filter(admin.eq(true)) - .order_by(published) - .load::(conn) - } - - pub fn banned(conn: &PgConnection) -> Result, Error> { - use super::user_view::user_fast::dsl::*; - use diesel::sql_types::{Nullable, Text}; - user_fast - .select(( - id, - actor_id, - name, - preferred_username, - avatar, - banner, - "".into_sql::>(), - matrix_user_id, - bio, - local, - admin, - banned, - show_avatars, - send_notifications_to_email, - published, - number_of_posts, - post_score, - number_of_comments, - comment_score, - )) - .filter(banned.eq(true)) - .load::(conn) - } - - // WARNING!!! this method WILL return sensitive user information and should only be called - // if the user requesting these details is also the authenticated user. - // please use get_user_secure to obtain user rows in most cases. - pub fn get_user_dangerous(conn: &PgConnection, user_id: i32) -> Result { - use super::user_view::user_fast::dsl::*; - user_fast.find(user_id).first::(conn) - } - - pub fn get_user_secure(conn: &PgConnection, user_id: i32) -> Result { - use super::user_view::user_fast::dsl::*; - use diesel::sql_types::{Nullable, Text}; - user_fast - .select(( - id, - actor_id, - name, - preferred_username, - avatar, - banner, - "".into_sql::>(), - matrix_user_id, - bio, - local, - admin, - banned, - show_avatars, - send_notifications_to_email, - published, - number_of_posts, - post_score, - number_of_comments, - comment_score, - )) - .find(user_id) - .first::(conn) - } -} diff --git a/lemmy_db/src/views/user_view.rs b/lemmy_db/src/views/user_view.rs index 33c441be6..be80179b2 100644 --- a/lemmy_db/src/views/user_view.rs +++ b/lemmy_db/src/views/user_view.rs @@ -1,9 +1,13 @@ use crate::{ aggregates::user_aggregates::UserAggregates, + fuzzy_search, + limit_and_offset, schema::{user_, user_aggregates}, user::{UserSafe, User_}, + MaybeOptional, + SortType, }; -use diesel::{result::Error, *}; +use diesel::{dsl::*, result::Error, *}; use serde::Serialize; #[derive(Debug, Serialize, Clone)] @@ -60,6 +64,140 @@ impl UserViewSafe { } } +mod join_types { + use crate::schema::{user_, user_aggregates}; + use diesel::{ + pg::Pg, + query_builder::BoxedSelectStatement, + query_source::joins::{Inner, Join, JoinOn}, + sql_types::*, + }; + + /// TODO awful, but necessary because of the boxed join + pub(super) type BoxedUserJoin<'a> = BoxedSelectStatement< + 'a, + ( + ( + Integer, + Text, + Nullable, + Text, + Nullable, + Nullable, + diesel::sql_types::Bool, + Bool, + Timestamp, + Nullable, + Bool, + Text, + SmallInt, + SmallInt, + Text, + Bool, + Bool, + Nullable, + Text, + Nullable, + Bool, + Nullable, + Nullable, + Timestamp, + Nullable, + Bool, + ), + (Integer, Integer, BigInt, BigInt, BigInt, BigInt), + ), + JoinOn< + Join, + diesel::expression::operators::Eq< + diesel::expression::nullable::Nullable, + diesel::expression::nullable::Nullable, + >, + >, + Pg, + >; +} + +pub struct UserQueryBuilder<'a> { + conn: &'a PgConnection, + query: join_types::BoxedUserJoin<'a>, + sort: &'a SortType, + page: Option, + limit: Option, +} + +impl<'a> UserQueryBuilder<'a> { + pub fn create(conn: &'a PgConnection) -> Self { + let query = user_::table.inner_join(user_aggregates::table).into_boxed(); + + UserQueryBuilder { + conn, + query, + sort: &SortType::Hot, + page: None, + limit: None, + } + } + + pub fn sort(mut self, sort: &'a SortType) -> Self { + self.sort = sort; + self + } + + pub fn search_term>(mut self, search_term: T) -> Self { + if let Some(search_term) = search_term.get_optional() { + self.query = self + .query + .filter(user_::name.ilike(fuzzy_search(&search_term))); + } + self + } + + pub fn page>(mut self, page: T) -> Self { + self.page = page.get_optional(); + self + } + + pub fn limit>(mut self, limit: T) -> Self { + self.limit = limit.get_optional(); + self + } + + pub fn list(self) -> Result, Error> { + let mut query = self.query; + + query = match self.sort { + SortType::Hot => query + .order_by(user_aggregates::comment_score.desc()) + .then_order_by(user_::published.desc()), + SortType::Active => query + .order_by(user_aggregates::comment_score.desc()) + .then_order_by(user_::published.desc()), + SortType::New => query.order_by(user_::published.desc()), + SortType::TopAll => query.order_by(user_aggregates::comment_score.desc()), + SortType::TopYear => query + .filter(user_::published.gt(now - 1.years())) + .order_by(user_aggregates::comment_score.desc()), + SortType::TopMonth => query + .filter(user_::published.gt(now - 1.months())) + .order_by(user_aggregates::comment_score.desc()), + SortType::TopWeek => query + .filter(user_::published.gt(now - 1.weeks())) + .order_by(user_aggregates::comment_score.desc()), + SortType::TopDay => query + .filter(user_::published.gt(now - 1.days())) + .order_by(user_aggregates::comment_score.desc()), + }; + + let (limit, offset) = limit_and_offset(self.page, self.limit); + query = query.limit(limit).offset(offset); + + let res = query.load::<(User_, UserAggregates)>(self.conn)?; + + Ok(vec_to_user_view_safe(res)) + } +} + fn vec_to_user_view_safe(users: Vec<(User_, UserAggregates)>) -> Vec { users .iter() diff --git a/lemmy_structs/src/community.rs b/lemmy_structs/src/community.rs index 3535c05a9..7db71c953 100644 --- a/lemmy_structs/src/community.rs +++ b/lemmy_structs/src/community.rs @@ -1,6 +1,6 @@ use lemmy_db::{ community_view::{CommunityFollowerView, CommunityModeratorView, CommunityView}, - user_view::UserView, + views::user_view::UserViewSafe, }; use serde::{Deserialize, Serialize}; @@ -61,7 +61,7 @@ pub struct BanFromCommunity { #[derive(Serialize, Clone)] pub struct BanFromCommunityResponse { - pub user: UserView, + pub user: UserViewSafe, pub banned: bool, } diff --git a/lemmy_structs/src/site.rs b/lemmy_structs/src/site.rs index dbbb37c4c..6dfa518bd 100644 --- a/lemmy_structs/src/site.rs +++ b/lemmy_structs/src/site.rs @@ -6,8 +6,7 @@ use lemmy_db::{ moderator_views::*, post_view::*, user::*, - user_view::*, - views::site_view::SiteView, + views::{site_view::SiteView, user_view::UserViewSafe}, }; use serde::{Deserialize, Serialize}; @@ -37,7 +36,7 @@ pub struct SearchResponse { pub comments: Vec, pub posts: Vec, pub communities: Vec, - pub users: Vec, + pub users: Vec, } #[derive(Deserialize)] @@ -100,8 +99,8 @@ pub struct SiteResponse { pub struct GetSiteResponse { pub site: Option, // Because the site might not be set up yet pub counts: SiteAggregates, - pub admins: Vec, - pub banned: Vec, + pub admins: Vec, + pub banned: Vec, pub online: usize, pub version: String, pub my_user: Option, diff --git a/lemmy_structs/src/user.rs b/lemmy_structs/src/user.rs index bf4a36286..93f929404 100644 --- a/lemmy_structs/src/user.rs +++ b/lemmy_structs/src/user.rs @@ -4,7 +4,7 @@ use lemmy_db::{ post_view::PostView, private_message_view::PrivateMessageView, user_mention_view::UserMentionView, - user_view::UserView, + views::user_view::{UserViewDangerous, UserViewSafe}, }; use serde::{Deserialize, Serialize}; @@ -81,7 +81,8 @@ pub struct GetUserDetails { #[derive(Serialize)] pub struct GetUserDetailsResponse { - pub user: UserView, + pub user: Option, + pub user_dangerous: Option, pub follows: Vec, pub moderates: Vec, pub comments: Vec, @@ -112,7 +113,7 @@ pub struct AddAdmin { #[derive(Serialize, Clone)] pub struct AddAdminResponse { - pub admins: Vec, + pub admins: Vec, } #[derive(Deserialize)] @@ -127,7 +128,7 @@ pub struct BanUser { #[derive(Serialize, Clone)] pub struct BanUserResponse { - pub user: UserView, + pub user: UserViewSafe, pub banned: bool, }