CONDSTORE

[superboum]

Progress

• CONDSTORE activation
  • ENABLE COMMAND: CONDSTORE (already added by you some times ago)
• Response
  • SELECT/EXAMINE COMMAND: HIGHESTMODSEQ Status Code (RFC g. add.)
  • SELECT/EXAMINE COMMAND: NOMODSEQ Status Code (RFC g. add.)
  • STATUS COMMAND HIGHESTMODSEQ Response parameter to the STATUS Response (RFC h. add.)
  • FETCH COMMAND MODSEQ data item (RFC c. add)
  • STORE COMMAND MODIFIED STATUS RESPONSE CODE (RFC b. add.)
  • SEARCH COMMAND Extend search response syntax (RFC f. add.)
• Modifiers
  • SELECT/EXAMINE: CONDSTORE MODIFIER (RFC h. add.)
  • FETCH COMMAND: CHANGEDSINCE modifier (RFC d. add.)
  • STORE COMMAND: UNCHANGEDSINCE modifier (RFC a. add.)
• Query
  • FETCH COMMAND: MODSEQ data item name (RFC c. add.)
  • SEARCH COMMAND: MODSEQ (RFC e. add.)

About this first commit

Hey Damian, I am trying to implement in a clean way condstore for imap-codec. I wanted to start with a scope as small as possible, that's why I have a commit adding support only for the status codes used by condstore. I propose you review this commit in depth, pinpoint all the problems with your conventions/standard, so I can backport the rest in a proper way?

Some questions

As for now, I am validating my code with:

cargo build --features ext_condstore_qresync
cargo test --features ext_condstore_qresync
cargo clippy
cargo fmt

Is it ok? Is it normal that I have warnings when running cargo fmt? Do you know why the imap-types/src/core.rs file has been impacted by my cargo fmt despite the fact that I have not edited it?

Also, another question: I created a mod_sequence_value parser that use your number64 parser. I did not name it nz_number64 despite the same logic for NonZeroU32 being name nz_number as it seems you want to follow the ABNF grammar, and in the grammar, it's called mod-sequence-value. Am I correct?

Also, you said at FOSDEM '24 that often examples given in the RFC are wrong. It appears that HIGHESTMODSEQ is given without a text following the code. Reading your code let me guess that's wrong. For example:

      S: * 1 RECENT
      S: * OK [UNSEEN 12] Message 12 is first unseen
      S: * OK [UIDVALIDITY 3857529045] UIDs valid
      S: * OK [UIDNEXT 4392] Predicted next UID
      S: * FLAGS (\Answered \Flagged \Deleted \Seen \Draft)
      S: * OK [PERMANENTFLAGS (\Deleted \Seen \*)] Limited
      S: * OK [HIGHESTMODSEQ 715194045007]
      S: A142 OK [READ-WRITE] SELECT completed

Dovecot correctly adds the text Highest after the code:

      S: * OK [HIGHESTMODSEQ 715194045007] Highest

I don't know if it qualifies for your IMAP defects repository.
Also feel free to edit this pull request directly if needed.

[coveralls]

Pull Request Test Coverage Report for Build 7827660241

• -10 of 38 (73.68%) changed or added relevant lines in 3 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.07%) to 92.988%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
imap-codec/src/codec/encode.rs	0	10	0.0%

Files with Coverage Reduction	New Missed Lines	%
imap-codec/src/codec/encode.rs	1	89.71%

Totals
Change from base Build 7784490136:	-0.07%
Covered Lines:	9574
Relevant Lines:	10296

---

💛 - Coveralls

[superboum]

It appears that cargo fmt and most commands should be run with the nightly toolchain. For the record, on NixOS, I don't use rustup, a nightly toolchain can be obtained with the following shell.nix:

{ pkgs ? import <nixpkgs> {} }:
let
  fenix = import (fetchTarball "https://github.com/nix-community/fenix/archive/main.tar.gz") { };
in
pkgs.mkShell {
  # nativeBuildInputs is usually what you want -- tools you need to run
  nativeBuildInputs = with pkgs.buildPackages; [
    fenix.complete.toolchain
  ];
}

So now formatting should pass hopefully.

[duesee]

It appears that cargo fmt and most commands should be run with the nightly toolchain.

It should generally only be cargo +nightly fmt (due to some options in rustfmt.toml) and fuzzing. Everything else is expected to work on stable.

For the record, on NixOS, I don't use rustup, a nightly toolchain can be obtained with the following shell.nix:

Do you think it would be beneficial to add this file to the repository? I'm open to anything that could improve the life of our contributors :-) Maybe we should add a snippet in CONTRIBUTING.md as well ...

[duesee]

Hey Quentin, your PR looks great! I have only one question so far regarding the RFC...

[duesee]

👍🏻

[duesee]

Here, I stopped for a moment ...

The RFC tells ...

resp-text-code      =/ "HIGHESTMODSEQ" SP mod-sequence-value /
                       "NOMODSEQ" /
                       "MODIFIED" SP set

... w/o saying what set is. Do you have more information on that?

[duesee]

Ahhh https://www.rfc-editor.org/errata/eid3506

[duesee]

Hm... so I guess it should be SequenceSet instead of Vec1<...>.

Yeah... RFC 7162 has sequence-set. You can use the type and sequence_set parser. Should be a simple change :-)