1737203 - nsIMsgCopyService.copyMessages() works even when destination folder has no database.

Status: ASSIGNED

(MailNews Core :: General, task)

Description

[Ben Campbell]

Probably best illustrated by test_copyToInvalidDB.js - it:

1. creates folder1 and folder2, containing some test messages
2. closes the db for one folder2, and deletes the .msf file entirely
3. moves a message from folder1 to folder2 (using nsIMsgCopyService.copyMessages())
4. triggers a reparse of folder2, rebuilding its db from scratch.

I'd rather expect step 3 to fail, but it currently doesn't.

My gut feeling is that this is a bad thing.
From the outside, it seems like folders should always have a valid database associated with them. And if they don't, such operations should fail.
This isn't to say that folders must always have databases resident in memory wasting space (we already do lazy DB loading)... but outside code should never have to deal with missing databases.
There's currently way too much brittleness and voodoo involving folders and databases (see Bug 1736931).

I'm prepared to be convinced I'm wrong on this, and there's a good reason why such folder operations should work even if there is no database... but at the moment I just can't see it.

Comment 1

[Ben Campbell]

Attachment: Bug 1737203 - Clarify state of msgDatabase and .msf in test_copyToInvalidDB.js. r=mkmelin

Adds a couple of asserts to make explicit the conditions before and after
moving a message into a folder with a dud msgDatabase.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7fccc19e4bf7
Clarify state of msgDatabase and .msf in test_copyToInvalidDB.js. r=mkmelin

Comment 3

[Wayne Mery (:wsmwk)]

Is this also to go into version 91?

Comment 4

[Ben Campbell]

(In reply to Wayne Mery (:wsmwk) from comment #3)

Is this also to go into version 91?

I don't see real reason for it to - this bug was just me trying to figure out and nail down some aspects of the architecture. No behavioural changes.

Comment 5

[Ben Campbell]

Had to think about this a little as part of Bug 1857450. Here are some notes:

So, copyMessages will proceed even though there is no database. And there will be no database until the database file is rebuilt (eg reparse mbox for local folders, or re-download all the headers for IMAP/news folders).

I think the options are:

1. make copyMessages trigger a folder reparse, and wait until it's done before continuing with the copy.
2. make copyMessages fail immediately.

I vote for 2, because it's much saner and simpler, and I don't think we should ever have missing databases anyway :-)
Ought to be an easy change, but obviously there'll be some code somewhere which totally relies on the current behaviour and requires some system to be completely rewritten to cope with it...