Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[r/boards] initialize pagination #3200

Closed
6 tasks
salmad3 opened this issue Nov 26, 2024 · 4 comments
Closed
6 tasks

[r/boards] initialize pagination #3200

salmad3 opened this issue Nov 26, 2024 · 4 comments

Comments

@salmad3
Copy link
Member

salmad3 commented Nov 26, 2024

Context

Proper pagination ensures the boards realm doesn’t become unresponsive or generate excessively large responses. By limiting how many threads or comments are retrieved in each call, we avoid performance bottlenecks and potential denial-of-service via spammy or long-running queries. Pagination also provides a structured way for clients to navigate large lists of threads and comments.

It is a viable path to adapt the avl pager. If not, we'll have to implement our own.

Acceptance Criteria

  • Configurable Page Size

    • Introduce ThreadPageSize and CommentPageSize in BoardsRealmConfig, with default values (e.g., 10 or 20).
    • Expose the ability for an AdminDAO to adjust these defaults per realm or board.
  • Pagination Functions

    • Implement a function like ListThreads(boardName string, page int) []Thread to retrieve a slice of threads for the given board:
      1. Validates page >= 1; returns an error or empty slice if out-of-bounds.
      2. Uses ThreadPageSize to calculate the starting offset.
      3. Returns at most ThreadPageSize threads, sorted by creation time or a default ordering.
    • Implement a function like ListComments(threadID string, page int) []Comment to retrieve comments for a thread, similarly respecting CommentPageSize and default sort order (chronological or otherwise).
    Example
    func (br *BoardsRealm) ListThreads(boardName string, page int) ([]Thread, error) {
        if page < 1 {
            return nil, errors.New("invalid page number")
        }
        pageSize := br.Config.ThreadPageSize
        startIdx := (page - 1) * pageSize
    
        threads := br.getThreadsByBoard(boardName) // e.g., returns all threads in memory or from storage
        if startIdx >= len(threads) {
            return []Thread{}, nil // no more results
        }
    
        endIdx := startIdx + pageSize
        if endIdx > len(threads) {
            endIdx = len(threads)
        }
        return threads[startIdx:endIdx], nil
    }
  • Permission Checks (Optional)

    • Normally, listing threads or comments is publicly accessible. However, if the board or realm is restricted, ensure only permitted users can see them.
    • For an open board, no special permission check is required for pagination calls.
  • Hidden Posts

    • Ensure hidden or flagged threads and comments are either omitted or marked accordingly.
    • The pagination logic should skip or filter out items that are not visible to the current user. (Exact approach can be MVP-simplified to “fully hidden = not listed.”)
  • Consistency for Large Data

    • The MVP approach can rely on a straightforward in-memory or simple index approach.
    • For extremely large boards, a more optimized data structure may be needed, but is out-of-scope for the initial release.
  • Tests

    • Verify standard paging scenarios:
      1. Requesting first page returns up to pageSize items.
      2. Requesting second page returns the next batch.
      3. Requesting a page beyond total results returns an empty slice (or minimal results).
    • Confirm correct ordering (chronological or by creation time).
    • Ensure hidden threads/comments do not appear if that is the chosen MVP approach.
    • Edge cases: negative page numbers, zero page size (if allowed), empty boards, boards with only one page of content.

Notes

  • Offset-based pagination is a simple solution for the MVP. In future releases, more advanced techniques (like “next-page tokens,” “cursor-based paging,” or “branching for nested comment trees”) may be explored.
  • If comments are deeply nested, the MVP may still return them in a flat list sorted by date. Later improvements can refine tree-based pagination or partial sub-thread retrieval.
  • Consider hooking into the DAO structure for per-board overrides of ThreadPageSize and CommentPageSize.
  • For extremely active boards, advanced indexing or storing threads/comments in specialized data structures might eventually be necessary, but the immediate MVP will focus on correctness and basic performance.
@jeronimoalbi
Copy link
Member

@salmad3 I believe this issue is not valid anymore, requirements changed so now boards are not listed and threads are listed but in a top ranking.

The top ranking of threads might need pagination but if so that could maybe be part of the ranking issue or a completely separate one.

@salmad3
Copy link
Member Author

salmad3 commented Jan 10, 2025

@jeronimoalbi description updated to reflect the latest criteria.

@salmad3
Copy link
Member Author

salmad3 commented Jan 14, 2025

[08-01-2025]:

We settled on having basic pagination in the first version. Without it, any spam or large data sets (e.g., many comments in a thread) could render a page non-functional.

  • The same paging approach applies to boards, threads, and comments—no exceptions.
  • Paginating heavy content ensures the system remains responsive and reliable, especially under high load.
  • Deep or nested comment trees may require trimming strategies.
  • Implementing pagination in the first version avoids costly overhauls.

@salmad3
Copy link
Member Author

salmad3 commented Jan 14, 2025

@x1unix will revisit to replace #3216

jeronimoalbi pushed a commit that referenced this issue Jan 27, 2025
Implement pagination for boards, threads and replies.

Closes #3200, #3539

CC @jeronimoalbi @salmad3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants