Improve MessageStoreLevel.query() by modifying filters based on the pagination cursor

[thehenrytsai]

Background:

At present, even when a pagination cursor is provided, the code executes the same query as if no cursor was supplied, and then trims the resulting array using the cursor.

Task Details:

A potential enhancement could be to adjust the given filters themselves before executing them. This could be done by looking up the value of the corresponding sorted property of message indicated by the cursor. This way, there would be no preceding entries to trim in the resulting array.

[KshitijBharde]

Hi @thehenrytsai, I would like to pick up this issue.

[jpowell96]

@KshitijBharde I'd be interested in picking this up too. Would you be open to collaborating on this one?

[jpowell96]

@thehenrytsai Just a clarifying question about the Pagination object in message-types.ts?

As is, the Pagination type provides a messageCid:

export type Pagination = {
  messageCid?: string
  limit?: number
};

I see that it's used as the start index when you slice the query results in message-store-level.ts.

When you say, "adjust the given filters", would that mean creating a filter based on the messageCid in the Pagination object so that it's considered when querying the index?

Finally, I've also seen different implementations of cursor pagination. In some implementations a user provides a "before" or "after" field that is the cursor you to use for pagination. It often looks something like this:

export type Pagination = {
  beforeMessageCid?: string
  afterMessageCid?: string
  limit?: number
}

With this Pagination type, if I wanted to return the 10 records after afterMessageCid "abc123", my pagination object would look like:

pagination: Pagination = {
  afterMessageCid: "abc123",
  limit: 10
}

This approach can be more complicated though. This is my first time in this repository, so want to clarify the direction you wanted to take.

[EbonyLouis]

@jpowell96 that's a great idea, @KshitijBharde would you be open to collaborating? If you both would like to work on it together we have a hack-together channel in discord I can give you both a stage to work on it.

[KshitijBharde]

@EbonyLouis @jpowell96 Absolutely! I'm open to collaborating on this one. Let's connect and discuss how we can work together to make it happen.

[thehenrytsai]

@jpowell96, really good questions! Shows you definitely understand the task and code at hand!

would that mean creating a filter based on the messageCid in the Pagination object so that it's considered when querying the index?

Yes, you are spot on, right now we "slice" the results AFTER a less efficient query is executed. If we are able to exclude the unnecessary messages with a query, there would be no need to slice the results.

The call out on beforeMessageCid and afterMessageCid is valid, right now it is implied and dependent on "sort order" that the query, I'd imagine a bulk of this PR would have to take into account if the sort order (whether it's descending or ascending). I am open to renames or addition to a mutually exclusive property (beforeMessageCid/afterMessageCid) if everyone thinks if makes everything clearer, though you'll still need to look at the query in the end to look at what property is being sorted.

Adding @LiranCohen in case he has some insights/thoughts/opinions on the potential renames.

[LiranCohen]

Agreed with @thehenrytsai on the various call outs, as long as it takes into account the higher-level sort property I am open to the before/after naming within the query.

[EbonyLouis]

🥳 @KshitijBharde what's your discord name? if you haven't joined yet here's the link and I can get you both set up.

[KshitijBharde]

@EbonyLouis here's my discord name: kshitij#3287

[EbonyLouis]

@KshitijBharde, @jpowell96 you're both set up in discord feel free to message me with any questions my username is EbonyL-TBD

Happy Hacking 🍁

[LiranCohen]

@KshitijBharde @jpowell96 After speaking with @thehenrytsai today it seems that this isn't a great issue for hacktoberfest after all. It is a little more involved and we are updating some things internally to IndexLevel that would conflict with this work.

Apologies for any spent effort. Hope to see your contributions on some of the other issues!

cc: @EbonyLouis

[LiranCohen]

This was addressed by:
#625