On 21/07/14 20:18, Jan Kundrát wrote:
> 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
> }
Absolutely I will do that tonight.
>
> I agree that using one class for IMAP and another one for SMTP sounds
> good.
That's a good idea. I was actually referring to using separate classes
for DNS SRV, Mozilla ISP and internal config file configuration. This
way each method could be spawned simultaneously and upon receiving a
configuration result from one, the remaining could be cancelled.
However, linking to your later point, it might be better to query each
sequentially, so that, for example, the ISP DB isn't queried unless DNS
SRV fails to return anything.
>
> 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.
That's definitely something I'd like to look into. I've not written unit
tests before but I'll do my best before taking up your valuable time :)
>
> 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.
This is definitely a good point. On another, related note, do we need to
check the SRV lookup and then ask the user if they want to use the ISPDB
for privacy reasons? In some ways it presents a small usability road
bump to the average user, but that might be preferable to automatically
sending the user's email address to a third-party.
>
> 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.
Shall do.
>
> With kind regards,
> Jan
>
