stalwartlabs / mail-server

Secure & Modern All-in-One Mail Server (IMAP, JMAP, POP3, SMTP)
https://stalw.art
3.8k stars 145 forks source link

[bug]: sort doesn't work in Email/query with collapseThreads=true #224

Open joostvanzwieten opened 6 months ago

joostvanzwieten commented 6 months ago

What happened?

I've been trying out the JMAP interface of the mail server and noticed that sort of the Email/query method doesn't do anything in combination with collapseThreads=true. Sorting works as expected without collapsing the threads. I've also tried Ltt.rs and noticed that the emails are displayed in insertion order, not by date, despite Ltt.rs requesting threads sorted on receivedAt.

How can we reproduce the problem?

To reproduce, you can try the following query (with appropriate accountId):

{
  "methodCalls": [
    [
      "Email/query",
      {
        "collapseThreads": true,
        "accountId": "a",
        "limit": 20,
        "sort": [
          {
            "isAscending": false,
            "property": "receivedAt"
          }
        ]
      },
      "0"
    ],
    [
      "Email/get",
      {
        "accountId": "a",
        "#ids": {
          "resultOf": "0",
          "name": "Email/query",
          "path": "/ids"
        },
        "properties": [
          "threadId"
        ]
      },
      "1"
    ],
    [
      "Thread/get",
      {
        "accountId": "a",
        "#ids": {
          "resultOf": "1",
          "name": "Email/get",
          "path": "/list/*/threadId"
        }
      },
      "2"
    ],
    [
      "Email/get",
      {
        "accountId": "a",
        "#ids": {
          "resultOf": "2",
          "name": "Thread/get",
          "path": "/list/*/emailIds"
        },
        "properties": [
          "id",
          "threadId",
          "receivedAt"
        ]
      },
      "3"
    ]
  ],
  "using": [
    "urn:ietf:params:jmap:core",
    "urn:ietf:params:jmap:mail"
  ]
}

For me this query returns the emails in insertion order, both with ascending and descending order. Setting collapseThreads to false gives the expected order.

Version

v0.5.x

What database are you using?

RocksDB

What blob storage are you using?

Filesystem

Where is your directory located?

Internal

What operating system are you using?

Linux

Relevant log output

No response

Code of Conduct

joostvanzwieten commented 5 months ago

I think I found the culprit. In crates/store/src/query/sort.rs method Store::sort there are three variants for sorting messages. The first applies to a single comparator and without collapsed threads, the second to multiple comparators and the last to the remainder (including a single comparator and collapsed threads). The third variant doesn't sort the messages. My problem is solved by using the second variant also for a single comparator, thus changing line 137 to

        } else if comparators.len() >= 1 {

I've briefly investigated adding support for filtering out duplicate prefixes in the first variant, but this requires async closures, which Store::iterate doesn't support.

mdecimus commented 5 months ago

Thanks, this is going to be reviewed once the webadmin is released.