KDav without KIO

[Christian Mollekopf]

On Mon, Jun 26, 2017, at 10:16 AM, Daniel Vrátil wrote:
> On Sunday, June 25, 2017 5:19:13 PM CEST Christian Mollekopf wrote:
> > Hi,
> 
> Hi,
> 
> > 
> > In order to resolve https://phabricator.kde.org/T5795 and finally get a
> > sustainable solution for DAV access for Kube/Sink I have ported KDav
> > from KIO to something based on https://github.com/mhaller/qwebdavlib
> > respectively QNetworkAccessManager.
> > 
> > You can find the code in the dev/nokio branch (I had to squash all
> > commits because of commits with non-unix line endings that could not be
> > pushed).
> > 
> > In it's current state all tests pass and it does what it needs to for
> > the time being. Some cleanup is still required, but functionally it
> > seems complete to me.
> > 
> > My question to you is if you agree with this direction and would like to
> > have that patch in kdav master, or if you'd like to stick with what's
> > currently in master.
> 
> I'm fine with merging this into kdav master, assuming the issues I raised
> below are fixed first. Also please make sure the DAV resource still works and
> haven't lost any functionality (especially SSL and proxy handling).

I'm not even building kdepim, so I can't offer that, sorry. Perhaps
Sandro can given he's the one that ported the resource to kdav.

> 
> From a quick glance at a code:
> 
> Since QWebDav is compeltely internalized to KDavManager, I don't see any
> way to:
> 1) handle SSL errors - you don't connect to
> QWebDav::checkSslCertificate() or 
> expose the signal through public API, so applications cannot show a
> dialog to confirm the SSL error, thus you won't be able to use KDAV against servers
> with e.g. self-signed certificates.
> 2) configure proxy - previously KIO would handle this automagically, now
> we need a way to pass QNetworkProxy settings to QWebDav's QNAM.
> 

I think our best bet to solve those two is to remove any error handling
from QWebDav and expose
the QNetworkAccessManager via some static API.

> You seem to be leaking all the QNetworkReplies (from QNAM docs: After the 
> request has finished, it is the responsibility of the user to delete the 
> QNetworkReply object at an appropriate time. Do not directly delete it
> inside the slot connected to finished(). You can use the deleteLater() function)

This should be handled in qwebdav.cpp via replyFinished() ->
replyDeleteLater().

> Also make sure to use  qCDebug/qCWarning instead of qDebug/qWarning 
> everywhere.

All debug statements are depending on DEBUG_WEBDAV to be defined, which
we don't, so that shouldn't be an issue for now.

Thanks for your feedback!

Cheers,
Christian