Repository: LemmyNet/lemmy (language: rust)

Issue:
Lots of redundant data returned by comment API
### Issue Summary

Lemmy API sends tons of redundant data. For each comment, the entire containing post and community are also returned, even if they are all the same.

### Steps to Reproduce

Go to https://feddit.de/api/v3/comment/list?post_id=745959&type_=All

### Technical details

`CommentView` contains these fields and they are not optional. They are of course needed if you are listing comments for a community, but when listing by post_id, they are redundant.



Discussion / hints:
Lots of reasons that the `CommentView` contains lots of extra data.

Take the profile page for instance: many comments from one person, but on different communities. 

![Screenshot_2023-06-07-20-42-29-518_net slions fulguris full fdroid](https://github.com/LemmyNet/lemmy/assets/930215/9b513818-cda7-4173-a77f-a6e6c39f1c2f)

Or the home page: lots of posts / comments from different communities.

Having lots of different `CommentView`s, which change forms depending on where they're selected from, would be a burden to maintain.

> Having lots of different CommentViews, which change forms depending on where they're selected from, would be a burden to maintain.

I understand that, however "show me the comments for this one post" is an extremely common case and deleting the post and community fields reduced size by half on a random thread I just tried, so I thought I'd at least mention it.
For sure. If anyone would like to add various CommentViews, I wouldn't be opposed, as long as they are well-typed.
Should the less redundant API be at `/post/comments`?
This will be a minor problem when we have compressed API responses (already merged) and optimized database queries.

This API should be made less redundant in /api/v4. The response should be an object with "users" and "posts" fields, which are both maps that are indexed with IDs. Each value in "posts" should have a "comments" field which is also indexed with IDs. Each value in "comments" should have its own "comments" field just like posts and only store the author's ID.

```
{
    "users": {
        1: {...}
    },
    "posts": {
        2: {
            ...,
            "creator_id": 1,
            "comments": {
                3: {
                    ...,
                    "creator_id": 1,
                    "comments": {}
                }
            }
        }
    }
}
```
How realistic or helpful would it be to implement a `?fields=field1,field2` parameter that determines which fields are returned? I know in some cases this can reduce server load/bandwidth usage (especially with nested data), and it would prevent the need for additional endpoints.
That sounds like a lot of effort to implement. And in the end clients would likely request all fields anyway to keep things simple. 
I would say this problem only affects `/api/v4/comment/list` when called with a `post_id`. In that case we can assume that the client already knows the post and the community. So to resolve the issue I would remove the post_id parameter, and instead add a new endpoint `/api/v4/post/list_comments?post_id`. 
Would this be okay?

`/comment/list/` - Returns full `CommentView`
`/comment/list/slim` - Returns a `CommentSlimView`

Because you never know what other places / locations, or UIs might want to use a slim version (IE not just the post list, but possibly other places). And I'd also like this to be a separate endpoint, so that the return type is predictable and can be output to typescript. And as stated in the other issue, I'd like to keep our primary objects at the top level.

Doesn't have to be `slim` of course, any name works.

Fix patch:
diff --git a/crates/api_common/src/comment.rs b/crates/api_common/src/comment.rs
index 0d416e9f05..fd2b57f433 100644
--- a/crates/api_common/src/comment.rs
+++ b/crates/api_common/src/comment.rs
@@ -3,7 +3,7 @@ use lemmy_db_schema::{
   CommentSortType,
   ListingType,
 };
-use lemmy_db_views::structs::{CommentView, VoteView};
+use lemmy_db_views::structs::{CommentSlimView, CommentView, VoteView};
 use serde::{Deserialize, Serialize};
 use serde_with::skip_serializing_none;
 #[cfg(feature = "full")]
@@ -144,6 +144,14 @@ pub struct GetCommentsResponse {
   pub comments: Vec<CommentView>,
 }
 
+#[derive(Debug, Serialize, Deserialize, Clone)]
+#[cfg_attr(feature = "full", derive(TS))]
+#[cfg_attr(feature = "full", ts(export))]
+/// A slimmer comment list response, without the post or community.
+pub struct GetCommentsSlimResponse {
+  pub comments: Vec<CommentSlimView>,
+}
+
 #[skip_serializing_none]
 #[derive(Debug, Serialize, Deserialize, Clone, Copy, Default, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "full", derive(TS))]
diff --git a/crates/apub/src/api/list_comments.rs b/crates/apub/src/api/list_comments.rs
index 45d2410733..3759d1a4bf 100644
--- a/crates/apub/src/api/list_comments.rs
+++ b/crates/apub/src/api/list_comments.rs
@@ -7,7 +7,7 @@ use crate::{
 use activitypub_federation::config::Data;
 use actix_web::web::{Json, Query};
 use lemmy_api_common::{
-  comment::{GetComments, GetCommentsResponse},
+  comment::{GetComments, GetCommentsResponse, GetCommentsSlimResponse},
   context::LemmyContext,
   utils::{check_conflicting_like_filters, check_private_instance},
 };
@@ -17,16 +17,17 @@ use lemmy_db_schema::{
 };
 use lemmy_db_views::{
   comment_view::CommentQuery,
-  structs::{LocalUserView, SiteView},
+  structs::{CommentView, LocalUserView, SiteView},
 };
 use lemmy_utils::error::{LemmyErrorExt, LemmyErrorType, LemmyResult};
 
+/// A common fetcher for both the CommentView, and CommentSlimView.
 #[tracing::instrument(skip(context))]
-pub async fn list_comments(
+async fn list_comments_common(
   data: Query<GetComments>,
   context: Data<LemmyContext>,
   local_user_view: Option<LocalUserView>,
-) -> LemmyResult<Json<GetCommentsResponse>> {
+) -> LemmyResult<Vec<CommentView>> {
   let site_view = SiteView::read_local(&mut context.pool()).await?;
   check_private_instance(&local_user_view, &site_view.local_site)?;
 
@@ -73,7 +74,7 @@ pub async fn list_comments(
   let post_id = data.post_id;
   let local_user = local_user_view.as_ref().map(|l| &l.local_user);
 
-  let comments = CommentQuery {
+  CommentQuery {
     listing_type,
     sort,
     max_depth,
@@ -89,7 +90,31 @@ pub async fn list_comments(
   }
   .list(&site_view.site, &mut context.pool())
   .await
-  .with_lemmy_type(LemmyErrorType::CouldntGetComments)?;
+  .with_lemmy_type(LemmyErrorType::CouldntGetComments)
+}
+
+#[tracing::instrument(skip(context))]
+pub async fn list_comments(
+  data: Query<GetComments>,
+  context: Data<LemmyContext>,
+  local_user_view: Option<LocalUserView>,
+) -> LemmyResult<Json<GetCommentsResponse>> {
+  let comments = list_comments_common(data, context, local_user_view).await?;
 
   Ok(Json(GetCommentsResponse { comments }))
 }
+
+#[tracing::instrument(skip(context))]
+pub async fn list_comments_slim(
+  data: Query<GetComments>,
+  context: Data<LemmyContext>,
+  local_user_view: Option<LocalUserView>,
+) -> LemmyResult<Json<GetCommentsSlimResponse>> {
+  let comments = list_comments_common(data, context, local_user_view)
+    .await?
+    .into_iter()
+    .map(CommentView::map_to_slim)
+    .collect();
+
+  Ok(Json(GetCommentsSlimResponse { comments }))
+}
diff --git a/crates/db_schema/src/lib.rs b/crates/db_schema/src/lib.rs
index c077f758b0..9c0f0ea0fa 100644
--- a/crates/db_schema/src/lib.rs
+++ b/crates/db_schema/src/lib.rs
@@ -292,13 +292,6 @@ pub enum FederationMode {
   Disable,
 }
 
-pub trait InternalToCombinedView {
-  type CombinedView;
-
-  /// Maps the combined DB row to an enum
-  fn map_to_enum(self) -> Option<Self::CombinedView>;
-}
-
 /// Wrapper for assert_eq! macro. Checks that vec matches the given length, and prints the
 /// vec on failure.
 #[macro_export]
diff --git a/crates/db_schema/src/traits.rs b/crates/db_schema/src/traits.rs
index bc30c6fb92..f010d3d1b6 100644
--- a/crates/db_schema/src/traits.rs
+++ b/crates/db_schema/src/traits.rs
@@ -197,3 +197,10 @@ pub trait ApubActor {
   where
     Self: Sized;
 }
+
+pub trait InternalToCombinedView {
+  type CombinedView;
+
+  /// Maps the combined DB row to an enum
+  fn map_to_enum(self) -> Option<Self::CombinedView>;
+}
diff --git a/crates/db_views/src/comment_view.rs b/crates/db_views/src/comment_view.rs
index 710a820e73..28e1f91bed 100644
--- a/crates/db_views/src/comment_view.rs
+++ b/crates/db_views/src/comment_view.rs
@@ -1,4 +1,4 @@
-use crate::structs::CommentView;
+use crate::structs::{CommentSlimView, CommentView};
 use diesel::{
   dsl::{exists, not},
   pg::Pg,
@@ -313,6 +313,22 @@ impl CommentView {
     }
     Ok(handle_deleted(res, is_admin))
   }
+
+  pub fn map_to_slim(self) -> CommentSlimView {
+    CommentSlimView {
+      comment: self.comment,
+      creator: self.creator,
+      counts: self.counts,
+      creator_banned_from_community: self.creator_banned_from_community,
+      banned_from_community: self.banned_from_community,
+      creator_is_moderator: self.creator_is_moderator,
+      creator_is_admin: self.creator_is_admin,
+      subscribed: self.subscribed,
+      saved: self.saved,
+      creator_blocked: self.creator_blocked,
+      my_vote: self.my_vote,
+    }
+  }
 }
 
 #[derive(Default)]
diff --git a/crates/db_views/src/person_content_combined_view.rs b/crates/db_views/src/person_content_combined_view.rs
index b20447b98e..d28dc7bdfb 100644
--- a/crates/db_views/src/person_content_combined_view.rs
+++ b/crates/db_views/src/person_content_combined_view.rs
@@ -41,8 +41,8 @@ use lemmy_db_schema::{
     combined::person_content::{person_content_combined_keys as key, PersonContentCombined},
     community::CommunityFollower,
   },
+  traits::InternalToCombinedView,
   utils::{actions, actions_alias, functions::coalesce, get_conn, DbPool},
-  InternalToCombinedView,
   PersonContentType,
 };
 use lemmy_utils::error::LemmyResult;
diff --git a/crates/db_views/src/person_saved_combined_view.rs b/crates/db_views/src/person_saved_combined_view.rs
index 4e1ce8df70..5900942790 100644
--- a/crates/db_views/src/person_saved_combined_view.rs
+++ b/crates/db_views/src/person_saved_combined_view.rs
@@ -38,8 +38,8 @@ use lemmy_db_schema::{
     combined::person_saved::{person_saved_combined_keys as key, PersonSavedCombined},
     community::CommunityFollower,
   },
+  traits::InternalToCombinedView,
   utils::{actions, actions_alias, functions::coalesce, get_conn, DbPool},
-  InternalToCombinedView,
   PersonContentType,
 };
 use lemmy_utils::error::LemmyResult;
diff --git a/crates/db_views/src/report_combined_view.rs b/crates/db_views/src/report_combined_view.rs
index c082d7f744..335fccb81d 100644
--- a/crates/db_views/src/report_combined_view.rs
+++ b/crates/db_views/src/report_combined_view.rs
@@ -47,8 +47,8 @@ use lemmy_db_schema::{
     combined::report::{report_combined_keys as key, ReportCombined},
     community::CommunityFollower,
   },
+  traits::InternalToCombinedView,
   utils::{actions, actions_alias, functions::coalesce, get_conn, DbPool, ReverseTimestampKey},
-  InternalToCombinedView,
 };
 use lemmy_utils::error::LemmyResult;
 
diff --git a/crates/db_views/src/structs.rs b/crates/db_views/src/structs.rs
index eedaa79805..43eba36c72 100644
--- a/crates/db_views/src/structs.rs
+++ b/crates/db_views/src/structs.rs
@@ -87,6 +87,27 @@ pub struct CommentView {
   pub my_vote: Option<i16>,
 }
 
+#[skip_serializing_none]
+#[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
+#[cfg_attr(feature = "full", derive(TS, Queryable))]
+#[cfg_attr(feature = "full", diesel(check_for_backend(diesel::pg::Pg)))]
+#[cfg_attr(feature = "full", ts(export))]
+/// A slimmer comment view, without the post, or community.
+pub struct CommentSlimView {
+  pub comment: Comment,
+  pub creator: Person,
+  pub counts: CommentAggregates,
+  pub creator_banned_from_community: bool,
+  pub banned_from_community: bool,
+  pub creator_is_moderator: bool,
+  pub creator_is_admin: bool,
+  pub subscribed: SubscribedType,
+  pub saved: bool,
+  pub creator_blocked: bool,
+  #[cfg_attr(feature = "full", ts(optional))]
+  pub my_vote: Option<i16>,
+}
+
 #[skip_serializing_none]
 #[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
 #[cfg_attr(feature = "full", derive(TS, Queryable))]
diff --git a/crates/db_views_actor/src/inbox_combined_view.rs b/crates/db_views_actor/src/inbox_combined_view.rs
index 5c04057423..8dec579e2f 100644
--- a/crates/db_views_actor/src/inbox_combined_view.rs
+++ b/crates/db_views_actor/src/inbox_combined_view.rs
@@ -46,9 +46,9 @@ use lemmy_db_schema::{
     combined::inbox::{inbox_combined_keys as key, InboxCombined},
     community::CommunityFollower,
   },
+  traits::InternalToCombinedView,
   utils::{actions, actions_alias, functions::coalesce, get_conn, DbPool},
   InboxDataType,
-  InternalToCombinedView,
 };
 use lemmy_utils::error::LemmyResult;
 
diff --git a/crates/db_views_moderator/src/modlog_combined_view.rs b/crates/db_views_moderator/src/modlog_combined_view.rs
index 4c6e619eab..cc349ec006 100644
--- a/crates/db_views_moderator/src/modlog_combined_view.rs
+++ b/crates/db_views_moderator/src/modlog_combined_view.rs
@@ -61,8 +61,8 @@ use lemmy_db_schema::{
     post,
   },
   source::combined::modlog::{modlog_combined_keys as key, ModlogCombined},
+  traits::InternalToCombinedView,
   utils::{get_conn, DbPool},
-  InternalToCombinedView,
   ModlogActionType,
 };
 use lemmy_utils::error::LemmyResult;
diff --git a/src/api_routes_v4.rs b/src/api_routes_v4.rs
index 852e868fd2..48710b0df4 100644
--- a/src/api_routes_v4.rs
+++ b/src/api_routes_v4.rs
@@ -142,7 +142,7 @@ use lemmy_api_crud::{
   },
 };
 use lemmy_apub::api::{
-  list_comments::list_comments,
+  list_comments::{list_comments, list_comments_slim},
   list_person_content::list_person_content,
   list_posts::list_posts,
   read_community::get_community,
@@ -277,6 +277,7 @@ pub fn config(cfg: &mut ServiceConfig, rate_limit: &RateLimitCell) {
           .route("/like/list", get().to(list_comment_likes))
           .route("/save", put().to(save_comment))
           .route("/list", get().to(list_comments))
+          .route("/list/slim", get().to(list_comments_slim))
           .route("/report", post().to(create_comment_report))
           .route("/report/resolve", put().to(resolve_comment_report)),
       )


Test patch: