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

Backend extensions #385

Closed
wants to merge 11 commits into from
Closed

Backend extensions #385

wants to merge 11 commits into from

Conversation

foxcpp
Copy link
Collaborator

@foxcpp foxcpp commented Aug 24, 2020

This is a proof-of-concept to start some work on #322. See last 2 commits. To demonstrate how it works I implemented server support for UIDPLUS extension and also add it to memory backend.

Based on #374. Will need rebasing to remove duplicate commits.

…to User

This is a preparation for dropping in-library updates dispatching.
The goal is to minimize creating Mailbox objects for commands that
are not related to the currently selected mailbox thus reserving
Mailbox to only represent currently selected mailbox.
It was observed that while using certain approaches to implement
stable sequence numbers view work done during mailbox selection
and one done on Status call overlaps. In order to avoid
complex contracts like "Status is always called immediately after
GetMailbox" or requesting backend to do duplicate work both calls
are merged so backend implementation can exploit that and
do work only once.
This permits backend to send appropriate updates for each
connection and maintain per-connection sequence number view
correctly.
EXAMINE command has a number of differences from
SELECT and implementing some of them requires
backend to be aware that EXAMINE command is used.
In particular, FETCH should always work as if
BODY.PEEK is used instead of BODY. That is,
\Seen flag should not be auto-added to messages.
CHECK command is largely unused and is often implemented
as a no-op. It is even removed in IMAP4rev2. This commit replaces
corresponding method with Poll().

Newly added argument for Poll is reserved for future extensions
that may require explicit polling but cannot allow sending
EXPUNGE updates.
This is necessary to generate EXISTS update as a result as required
("SHOULD") by RFC 3501 if the current mailbox is selected.
Was accidentally removed when moving CreateMessage.
Also it was never present for COPY so it was added there
along the way.

memory backend is also updated to conform with that and
return backend.ErrNoSuchMailbox.
…ions

See GH#322 for related discussion. This commit takes a slightly
different approach for interface design by using slices of opaque
values instead of a struct. This has a number of benefits, notably
backend can easily detect that it receives an "option" that it
cannot handle while in struct that would not be possible to
see new fields. This, however, creates an similar situation
but for go-imap. It is believed that it is not a significant
issue since we expect many backends and one go-imap :)

Interface changes are mostly applied to command affected by
RFC 4466 which defines an uniform framework for command
extensions.

In this design, it is important for go-imap to know the exact
set of extensions supported by backend beforehand (e.g.
which Options it can handle). SupportedExtensions() is
added to accommodate that. For consistency purposes, it should
also be used for extensions that can be easily checked using
type assertions (e.g. extensions adding brand new commands).
Written as a PoC for extendable backend interface.
@foxcpp
Copy link
Collaborator Author

foxcpp commented Aug 24, 2020

Probably it is also a good idea to merge #342 first into v1, then rebase dev onto master and #374 onto dev, ensuring all changes fitting together well and then only this. Gosh, that's a lot of PR dependencies.

linanh added a commit to linanh/go-imap that referenced this pull request Jul 7, 2021
@emersion
Copy link
Owner

emersion commented Apr 4, 2023

Closing because this is superseded by go-imap v2.

@emersion emersion closed this Apr 4, 2023
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