From 922ee6a2305d08ba06f3f2343c563cb825e410fc Mon Sep 17 00:00:00 2001 From: phiresky Date: Thu, 6 Jul 2023 14:44:26 +0200 Subject: [PATCH] improve admin and mod check to not do seq scans and return unnecessary data (#3483) * improve admin and mod check * fix clippy * move admin index to existing code * Revert "move admin index to existing code" This reverts commit d0c58d5f4021e1775d0c1d30d8df6c7df87557c4. * third attempt at the migration * fix formatting * rebuild --------- Co-authored-by: Dessalines --- .../src/community_moderator_view.rs | 21 +++++++++++++++++- crates/db_views_actor/src/community_view.rs | 22 +++---------------- crates/db_views_actor/src/person_view.rs | 11 ++++++++++ .../up.sql | 2 +- .../2023-07-05-000058_person-admin/down.sql | 2 ++ .../2023-07-05-000058_person-admin/up.sql | 2 ++ 6 files changed, 39 insertions(+), 21 deletions(-) create mode 100644 migrations/2023-07-05-000058_person-admin/down.sql create mode 100644 migrations/2023-07-05-000058_person-admin/up.sql diff --git a/crates/db_views_actor/src/community_moderator_view.rs b/crates/db_views_actor/src/community_moderator_view.rs index 63b4a5a5..113efe4b 100644 --- a/crates/db_views_actor/src/community_moderator_view.rs +++ b/crates/db_views_actor/src/community_moderator_view.rs @@ -1,5 +1,5 @@ use crate::structs::CommunityModeratorView; -use diesel::{result::Error, ExpressionMethods, QueryDsl}; +use diesel::{dsl::exists, result::Error, select, ExpressionMethods, QueryDsl}; use diesel_async::RunQueryDsl; use lemmy_db_schema::{ newtypes::{CommunityId, PersonId}, @@ -12,6 +12,25 @@ use lemmy_db_schema::{ type CommunityModeratorViewTuple = (Community, Person); impl CommunityModeratorView { + pub async fn is_community_moderator( + pool: &DbPool, + find_community_id: CommunityId, + find_person_id: PersonId, + ) -> Result { + use lemmy_db_schema::schema::community_moderator::dsl::{ + community_id, + community_moderator, + person_id, + }; + let conn = &mut get_conn(pool).await?; + select(exists( + community_moderator + .filter(community_id.eq(find_community_id)) + .filter(person_id.eq(find_person_id)), + )) + .get_result::(conn) + .await + } pub async fn for_community(pool: &DbPool, community_id: CommunityId) -> Result, Error> { let conn = &mut get_conn(pool).await?; let res = community_moderator::table diff --git a/crates/db_views_actor/src/community_view.rs b/crates/db_views_actor/src/community_view.rs index 899931c4..991c4cdd 100644 --- a/crates/db_views_actor/src/community_view.rs +++ b/crates/db_views_actor/src/community_view.rs @@ -90,29 +90,13 @@ impl CommunityView { person_id: PersonId, community_id: CommunityId, ) -> Result { - let is_mod = CommunityModeratorView::for_community(pool, community_id) - .await - .map(|v| { - v.into_iter() - .map(|m| m.moderator.id) - .collect::>() - }) - .unwrap_or_default() - .contains(&person_id); + let is_mod = + CommunityModeratorView::is_community_moderator(pool, community_id, person_id).await?; if is_mod { return Ok(true); } - let is_admin = PersonView::admins(pool) - .await - .map(|v| { - v.into_iter() - .map(|a| a.person.id) - .collect::>() - }) - .unwrap_or_default() - .contains(&person_id); - Ok(is_admin) + PersonView::is_admin(pool, person_id).await } } diff --git a/crates/db_views_actor/src/person_view.rs b/crates/db_views_actor/src/person_view.rs index 2a7a2ce7..06215123 100644 --- a/crates/db_views_actor/src/person_view.rs +++ b/crates/db_views_actor/src/person_view.rs @@ -11,6 +11,7 @@ use diesel_async::RunQueryDsl; use lemmy_db_schema::{ aggregates::structs::PersonAggregates, newtypes::PersonId, + schema, schema::{person, person_aggregates}, source::person::Person, traits::JoinView, @@ -34,6 +35,16 @@ impl PersonView { Ok(Self::from_tuple(res)) } + pub async fn is_admin(pool: &DbPool, person_id: PersonId) -> Result { + use schema::person::dsl::{admin, id, person}; + let conn = &mut get_conn(pool).await?; + let is_admin = person + .filter(id.eq(person_id)) + .select(admin) + .first::(conn) + .await?; + Ok(is_admin) + } pub async fn admins(pool: &DbPool) -> Result, Error> { let conn = &mut get_conn(pool).await?; let admins = person::table diff --git a/migrations/2023-07-04-153335_add_optimized_indexes/up.sql b/migrations/2023-07-04-153335_add_optimized_indexes/up.sql index c2911df7..c55ecee9 100644 --- a/migrations/2023-07-04-153335_add_optimized_indexes/up.sql +++ b/migrations/2023-07-04-153335_add_optimized_indexes/up.sql @@ -1,5 +1,5 @@ -- Create an admin person index -create index idx_person_admin on person (admin); +create index if not exists idx_person_admin on person (admin); -- Compound indexes, using featured_, then the other sorts, proved to be much faster -- Drop the old indexes diff --git a/migrations/2023-07-05-000058_person-admin/down.sql b/migrations/2023-07-05-000058_person-admin/down.sql new file mode 100644 index 00000000..b77e1747 --- /dev/null +++ b/migrations/2023-07-05-000058_person-admin/down.sql @@ -0,0 +1,2 @@ +drop index idx_person_admin; +create index idx_person_admin on person(admin); \ No newline at end of file diff --git a/migrations/2023-07-05-000058_person-admin/up.sql b/migrations/2023-07-05-000058_person-admin/up.sql new file mode 100644 index 00000000..c71052ec --- /dev/null +++ b/migrations/2023-07-05-000058_person-admin/up.sql @@ -0,0 +1,2 @@ +drop index if exists idx_person_admin; +create index idx_person_admin on person(admin) where admin; -- allow quickly finding all admins (PersonView::admins) \ No newline at end of file