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

go-imap v2 #260

Closed
emersion opened this issue Jun 1, 2019 · 6 comments
Closed

go-imap v2 #260

emersion opened this issue Jun 1, 2019 · 6 comments
Milestone

Comments

@emersion
Copy link
Owner

emersion commented Jun 1, 2019

go-imap v1 will be released without any major changes to the API. But it's time to cleanup the mess for v2:

go-imap extensions system is a kind of mess. It is hard to extend existing commands, I think this we might want to reconsider how it should work, taking into account the number of extensions that modify existing commands instead of adding new.

Originally posted by @foxcpp in #82 (comment)

@foxcpp
Copy link
Collaborator

foxcpp commented Jun 5, 2019

IIRC After some discussion on IRC, we agreed that the best way is to ditch modular design and implement all extensions in go-imap.

We could just a lot of arguments for all extensions that we might support... Then it will be compatibility disaster since the addition of the new arguments/return values to support new extensions will break it.

Basically, my current idea is to add structure arguments to each client/backend command that will let us pass all extension information. The same goes for return values.

For example this is client would pass UNCHANGEDSINCE (from CONDSTORE) argument to STORE:

_, err := cl.Store(seqSet, imap.FormatFlagsOp(imap.AddFlags, true), []string{"$foo"}, nil, imap.ExtCmdData{
  UnchangedSince: ...,
})

Note the additional return value, this is the structure containing possible extension responses, unused now.

@foxcpp
Copy link
Collaborator

foxcpp commented Jun 6, 2019

Also, the current storage backend interface forces backends to be stateless, making it impossible to implement \Recent flag handling and other things that require keeping track of opened mailboxes.

We need a way to explicitly tell backend when user opens a mailbox and how (EXAMINE vs SELECT). The same goes for the CLOSE command

type User interface {
  // ...
  OpenMailbox(name string, readOnly bool) (backend.Mailbox, error)
}
type Mailbox interface {
  // ...
  Close() error
}

@foxcpp
Copy link
Collaborator

foxcpp commented Jun 15, 2019

Side note: We probably want to mark the existing "extensions interface" in go-imap v1 as deprecated to indicate that we are getting rid of it.

@foxcpp
Copy link
Collaborator

foxcpp commented Jun 15, 2019

Another consideration for v2 backend interface:

Support for certain extensions may not be determined by simply type-asserting on a certain interface. This is what I workaround by adding EnableChildrenExt method in go-imap-sql and defining the ChildrenBackend interface with it. This is to ensure that the CHILDREN extension will be advertised only if used backend actually supports it.

Note that with possible go-imap v2 backend interface changes (similar to client interface changes) this will be the case for many more extensions.

The more generic solution for this problem is used in maddy codebase. The IMAPExtensions() method on storage backend. It returns a list of extensions actually supported by the used backend.
We might want to add something like that to the new backend interface, wdyt?

@emersion
Copy link
Owner Author

I've rewritten the client part in the v2 branch. I plan to work on the server part based off of @foxcpp's PR.

@emersion
Copy link
Owner Author

emersion commented Apr 4, 2023

The v2 branch is now mature enough to close this.

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

No branches or pull requests

2 participants