Refactor MboxParser and MboxMsgInputStream
Categories
(MailNews Core :: General, task)
Tracking
(Not tracked)
People
(Reporter: benc, Unassigned)
References
Details
When I collected the mbox handling into one place, I screwed up the architecture of the MboxParser and MboxMsgInputStream classes.
I started with MboxMsgInputStream, and made it own an mbox parser. This works great for reading single messages (which is the usual case).
But when we need to scan through the entire mbox (compaction, local folder repair), it becomes a complete pain and lots of hoop-jumping is required to handle special cases. We have to keep the same inputstream for multiple messages, and artificially kick it to continue onto the next message. We can't use a new inputstream for each message because we want to keep the state (and buffered data) of the underlying mbox parser owned by the stream.
The hoop jumping was tolerable originally, but as things changed it got more and more complicated.
(eg Bug 1926810 changed the parser so it would always fail if there wasn't a "From " marker at the read position. This caused problems with valid, empty mbox files - see Bug 1935124)
How to clean it up:
The mbox parser needs to be broken out from the MboxMsgInputStream.
The changes would all be confined inside nsMsgBrkMBoxStore, so nothing outside nsIMsgPluggableStore needs to be affected.
There are only two places where MboxMsgInputStream (and MboxParser) is created, so that's where I'd start from:
https://searchfox.org/comm-central/search?q=new+MboxMsgInputStream&path=&case=false®exp=false
What is the time frame of this change landing?
A week, a few weeks, a couple of months?
I am asking because I need to adjust my local patches. (I suffered from a severe bout of influenza since mid-December , and I started merging with C-C tree a few days ago, and already I ended up with many merge conflicts with the changes in the last few weeks.)
It took me a whole evening just to come up with a local source tree that can be compiled and I probably need another day or two to split the local changes into manageable patches.
The pace of changes these days is very fast (which is good IMHO), but it certainly needs constant efforts to keep up with the changes and make one's local patches in sync.
|
Reporter | |
Comment 2•1 month ago
|
||
It's not a massively high priority. Can't really see it happening within the next few months.
There'd be a lot of cleanup/simplification of MboxMsgInputStream (and a bit in nsMsgBrkMBoxStore, but there shouldn't be too much ripple-on effect outside that..
Bug 1714472 is probably likely to have more of an immediate impact. I plan to start trying to tidying up the nsIMsgPluggableStore interface over the next few months (so the store becomes just a store, and doesn't poke about with the folder or database like it does now).
(In reply to Ben Campbell from comment #2)
It's not a massively high priority. Can't really see it happening within the next few months.
There'd be a lot of cleanup/simplification ofMboxMsgInputStream(and a bit innsMsgBrkMBoxStore, but there shouldn't be too much ripple-on effect outside that..
Bug 1714472 is probably likely to have more of an immediate impact. I plan to start trying to tidying up thensIMsgPluggableStoreinterface over the next few months (so the store becomes just a store, and doesn't poke about with the folder or database like it does now).
Thank you for the clarification.
I am tidying up local patches in a manageable form.
I am using ”meld" for three way merge, but its user interface is not quite intuitive when a merge conflict occurs and I messed up the merge process somewhat. So manual cleanup has become necessary. :-(
Being able to look at the clean source code in searchfox.org is quite handy when local merge is really screwed up.
Description
•