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

User Content Feed Loader #34

Merged
merged 37 commits into from
Jul 27, 2024
Merged

User Content Feed Loader #34

merged 37 commits into from
Jul 27, 2024

Conversation

EricBAndrews
Copy link
Member

@EricBAndrews EricBAndrews commented Jul 24, 2024

This PR adds a feed loader for user content (mixed posts and comments, used in saved and profile).

For testing, you can use mlemgroup/mlem#1157, which implements the saved feed.

Due to the fact that the API does not provide a type-specific endpoint to load this content, we can't just use the parent/child loader setup; instead, I have implemented a similar architecture but using a much more lightweight "stream" in place of the child loaders. All of this is completely non-generic because as far as I know this is the only case where we have this API constraint, and for all other mixed feeds we can just use the parent/child architecture.

The basic loading concept works like this:

  • UserContentStream stores a list of items, their loading thresholds, and their stream cursor. Interaction with them is done through three methods:
    • addItems: adds new items to the stream. The stream handles updating the loading thresholds.
    • nextItemSortVal: functions identically to the child feed loader; returns the sort value of the next item in the stream, but does not affect the cursor
    • consumeNextItem: returns the next item in the stream and updates the cursor
    • The UserContentStream also provides the convenience needsMoreItems bool, which is true when the stream believes there are more items to load but has reached the end of its items.
  • UserContentFeedLoader maintains two UserContentStreams, one for posts and one for comments
  • To load more items, the UserContentFeedLoader performs the same stream merge operation as the parent/child architecture. Unlike a parent tracker, however, at each iteration the UserContentFeedLoader checks whether either child needs more items and, if so, triggers a load.

To support single-type profile feeds, UserContentFeedLoader boasts some additional modifications from the standard feed loading architecture:

  • loadIfThreshold supports checking a particular stream's thresholds. This is not currently used, but is designed to support single-item profile feeds.
  • The streams themselves are private, since they are directly and intimately managed by the UserContentFeedLoader and have lots of thread-unsafe operations. Single-type feeds are therefore exposed through the posts and comments computed vars.
  • To support refreshing without clearing on single-type feeds, two temporary streams are defined which supply the single-type streams while the main streams are reset. This is required because loading a fresh stream involves first clearing the children, then loading new items into them, then finally merging those items together into the parent stream.

This PR also makes a few minor changes:

  • MiddlewareConstants.infiniteLoadThresholdOffset is now 10 instead of -10. Arithmetic has been updated accordingly.
  • getContent now returns a tuple of (posts: [Post2], comments: [Comment2]) instead of a Person3, and takes parameters
  • FeedLoaderSortVal.published has been renamed to .new. This will let us support .old in the future.
  • FeedLoader thresholds are now handled using the new Thresholds struct.

@EricBAndrews EricBAndrews mentioned this pull request Jul 24, 2024
4 tasks
Copy link
Collaborator

@Sjmarf Sjmarf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! This is a clean solution 💯

@Sjmarf
Copy link
Collaborator

Sjmarf commented Jul 25, 2024

as far as I know this is the only case where we have this API constraint

There's also the Modlog

@EricBAndrews
Copy link
Member Author

as far as I know this is the only case where we have this API constraint

There's also the Modlog

Modlog is all a single endpoint, but you can specify the specific type to fetch so we can paginate independently--this is the only endpoint that forces you to get everything

@EricBAndrews EricBAndrews requested a review from Sjmarf July 25, 2024 20:37
@EricBAndrews EricBAndrews requested a review from Sjmarf July 25, 2024 21:23
Copy link
Collaborator

@Sjmarf Sjmarf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@EricBAndrews EricBAndrews merged commit 282be30 into master Jul 27, 2024
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

Successfully merging this pull request may close these issues.

2 participants