On Sunday, 20 July 2014 22:46:29 CEST, Matt Richardson wrote:
Having just given RFC 6186 a cursory read it doesn't look too difficult
to add in to the existing DNS record checks.
I am wondering, though, whether it might be best to separate each
individual method of auto-configuration into classes, with a control
class running through each method (either simultaneously or
sequentially) to return the configuration in the shortest possible time.
Sounds reasonable.
I would like the API of the settings object to use static properties as
opposed to the dynamic ones, though, i.e. something like this:
class ImapConfiguration: public QObject {
Q_PROPERTY(QString imapServer READ imapServer NOTIFY imapServerChanged)
// more properties for each corresponding property in ImapAccess
}
I agree that using one class for IMAP and another one for SMTP sounds good.
Then you'll need an adaptor which ties these settings into an ImapAccess.
The ImapAccess cannot be just created by this class, so let's use an
adaptor which simply sets ImapAccess' properties upon the
ImapConfiguration's done() signal. My vote is for putting this directly
integrated into the ImapConfiguration directly (=less boilerplate code).
The alternative would be having another extra class for the added benefit
of this being trivially modifiable into a standalone library without any
Trojita-specific ties. But doing that properly would require PIMPLing
anyway, so I don't think it's worth the hassle at this time.
So what about usage like this:
auto driver = new Autoconfiguration::Driver(this, ui->email->text());
driver->setTargetImapAccess(imapAccess);
connect(driver, SIGNAL(succeeded()), ui, SLOT(autodiscoveryOk()));
connect(driver, SIGNAL(failed(QString)), ui,
SLOT(autodiscoveryFailed(QString));
connect(driver, SIGNAL(done()), driver, SLOT(deleteLater()));
driver->start();
-> in other words, what you have as the class hodling the actual
configuration data is an implementation detail from the GUI's point of
view.
I would like to have reasonable testability. As some of the autobuild nodes
run on systems without network connectivity (and because having test
results depend on third-party servers is a bad idea), there must be
*something* which works on data "obtained through the network" (Qt's DNS
query info, or whatever). In the unit tests, this must be fed with locally
geenrated data. Apart from that, it would be cool to have a single test
case for the "whole stack", including the DNS query -- preferably for some
well-known e-mail service which is unlikely to go down, and test whether
the code is able to arrive to good data when queried. A cmake option
controlling this test would be great. I can help with this -- just saying
it here so that it doesn't get lost.
The Mozilla's autoconfig DB is a good thing, but it's a non-standard,
proprietary thing. I definitely prefer using the DNS-based SRV lookup over
this, so please make sure the code uses the SRV approach in preference, and
only if it fails, go ahead and check the ISPDB.
Finally, please use KDE's ReviewBoard when you have your patch ready as
nobody reads github pull requests for Trojita. And really finally --
thanks, it's appreciated that you're looking into this.
With kind regards,
Jan
--
Trojitá, a fast Qt IMAP e-mail client -- http://trojita.flaska.net/
