Open Bug 1714472 Opened 3 years ago Updated 6 months ago

[meta] Decouple nsIMsgPluggableStore from nsIMsgFolder and nsIMsgDatabase

Categories

(MailNews Core :: General, task)

Tracking

(Not tracked)

People

(Reporter: benc, Assigned: benc)

References

Details

(Keywords: meta)

nsIMsgPluggableStore implementations (mbox, maildir) should be more self contained and not know anything about folders or databases.

Currently, they are very intertwined with folder and db code, which makes it very hard to follow the logic of a lot of otherwise-simple operations, causes all kind of opportunities for subtle bugs, makes the mailstore conversion much harder than it needs to be, and makes unit testing a nightmare.

My planned approach is to refactor methods gradually - they don't all need to be done at once.

The guidelines I want to apply are:

  1. all references to messages in the store API should use the "storeToken" string, rather than nsIMsgDBHdr. (storeToken is the messages primary key in the store. It's a filename for maildir, and int offset for mbox. It's recorded in the nsIMsgDatabase).

  2. any operations which need to modify the database should instead return enough information that the caller can make the required changes instead. (eg if a new message is created, the store should return the new storeToken to the caller to insert into the database, rather than doing it itself)

  3. the store should not trigger any notifications (eg notifying folder listeners when messages have been moved into them). The responsibility for those sending those notifications shifts to the caller.

  4. folders inside the store should be referenced by a path string.

Why the percent-encoding? Wouldn't it make more sense to let the percent-encoding happen only when it's needed?

(In reply to Magnus Melin [:mkmelin] from comment #1)

Why the percent-encoding? Wouldn't it make more sense to let the percent-encoding happen only when it's needed?

That's an excellent question - it gets right into some of the corner-cases I'm trying to handle by default.
Here are a few thoughts on it:

  1. we're representing a hierarchical path, so we need a separator character. And '/' is the obvious choice. But I also want to support path components containing that separator character (ie folder names containing '/' - see Bug 29926, Bug 286523, Bug 358208, Bug 397875, Bug 436032, Bug 773579, Bug 63038 etc... :- ). RFC 3986 (the path part) and percent-encoding covers this nicely.
  2. it allows eventual direct compatibility with our folder URIs (ie the path portion of the folder URI could be passed directly to and from the mailstore). Dubious benefit, but the two uses represent the same hierarchy and face the same issues.
  3. We could use some other escaping scheme to handle the path separator, but we don't really have a good track record on that (eg Bug 1608318 :-).

Also, just to clarify: there still needs to be a mapping from the folder path to an actual filesystem path. For that I'd probably decode path components out to native encoding (utf-16 on windows, utf-8 everywhere else), and apply all the other rules (eg no AUX/PRN/CON etc on windows, be careful about folder names ending with ".sbd" or ".msf" (Bug 366789), folders called ".." etc etc etc).

(I'm currently leaning toward the idea of having the same file-naming rules across all platforms, so users can always transport their data between machines without any naming hassles... we'll see. There's always going to be someone on Linux who absolutely needs an "aaa" folder and an "AAA" folder!).

Keywords: meta
Depends on: 1890448
You need to log in before you can comment on or make changes to this bug.