Commit 4df06e0e authored by Daniel Vrátil's avatar Daniel Vrátil 🤖

Don't use LocalDataBaseManager as a singleton

Instead make LocalDatabaseManager an instance-based interface for a
singleton backend. The reason is that when running Kontact, both
Akregator, KMail and possibly others who use WebEngineViewer would
share the same instance of LocalDataBaseManager and connect to its
signal. This means that when you click on a link in KMail, KMail
requests URL check from LocalDataBaseManager singleton and the
singleton emits a signal, the signal is delivered to all
applications that are connected to it, not just KMail. This results
in each application opening the link.

This patch removes the singleton from LocalDataBaseManager and makes
each WebEngineView own its own instance of LocalDataBaseManager.
Internally, the actual implementation is still a singleton, however
the public interface is non-singleton, which means that each application
is connected to its own LocalDataBaseManager instance, thus avoiding
the issue described above.

Differential Review: https://phabricator.kde.org/D5577
parent 04f595fe
cmake_minimum_required(VERSION 3.0) cmake_minimum_required(VERSION 3.0)
set(PIM_VERSION "5.5.42") set(PIM_VERSION "5.5.43")
if (POLICY CMP0053) if (POLICY CMP0053)
cmake_policy(SET CMP0053 NEW) cmake_policy(SET CMP0053 NEW)
......
...@@ -24,11 +24,9 @@ ...@@ -24,11 +24,9 @@
#include <KSharedConfig> #include <KSharedConfig>
#include <QTest> #include <QTest>
extern MESSAGEVIEWER_EXPORT bool messageviewer_initialize_database;
ViewerGrantleeThemeSupportTest::ViewerGrantleeThemeSupportTest(QObject *parent) ViewerGrantleeThemeSupportTest::ViewerGrantleeThemeSupportTest(QObject *parent)
: QObject(parent) : QObject(parent)
{ {
messageviewer_initialize_database = false;
} }
ViewerGrantleeThemeSupportTest::~ViewerGrantleeThemeSupportTest() ViewerGrantleeThemeSupportTest::~ViewerGrantleeThemeSupportTest()
......
...@@ -21,10 +21,8 @@ ...@@ -21,10 +21,8 @@
#include <qtestmouse.h> #include <qtestmouse.h>
#include <KActionCollection> #include <KActionCollection>
extern MESSAGEVIEWER_EXPORT bool messageviewer_initialize_database;
ViewerTest::ViewerTest() ViewerTest::ViewerTest()
{ {
messageviewer_initialize_database = false;
} }
void ViewerTest::shouldHaveDefaultValuesOnCreation() void ViewerTest::shouldHaveDefaultValuesOnCreation()
......
...@@ -163,7 +163,6 @@ using namespace MessageViewer; ...@@ -163,7 +163,6 @@ using namespace MessageViewer;
using namespace MessageCore; using namespace MessageCore;
static QAtomicInt _k_attributeInitialized; static QAtomicInt _k_attributeInitialized;
MESSAGEVIEWER_EXPORT bool messageviewer_initialize_database = true;
template<typename Arg, typename R, typename C> template<typename Arg, typename R, typename C>
struct InvokeWrapper { struct InvokeWrapper {
...@@ -229,7 +228,8 @@ ViewerPrivate::ViewerPrivate(Viewer *aParent, QWidget *mainWindow, ...@@ -229,7 +228,8 @@ ViewerPrivate::ViewerPrivate(Viewer *aParent, QWidget *mainWindow,
mHeaderStyleMenuManager(nullptr), mHeaderStyleMenuManager(nullptr),
mViewerPluginToolManager(nullptr), mViewerPluginToolManager(nullptr),
mZoomActionMenu(nullptr), mZoomActionMenu(nullptr),
mCurrentPrinter(nullptr) mCurrentPrinter(nullptr),
mPhishingDatabase(nullptr)
{ {
mMimePartTree = nullptr; mMimePartTree = nullptr;
if (!mainWindow) { if (!mainWindow) {
...@@ -238,13 +238,11 @@ ViewerPrivate::ViewerPrivate(Viewer *aParent, QWidget *mainWindow, ...@@ -238,13 +238,11 @@ ViewerPrivate::ViewerPrivate(Viewer *aParent, QWidget *mainWindow,
if (_k_attributeInitialized.testAndSetAcquire(0, 1)) { if (_k_attributeInitialized.testAndSetAcquire(0, 1)) {
Akonadi::AttributeFactory::registerAttribute<MessageViewer::MessageDisplayFormatAttribute>(); Akonadi::AttributeFactory::registerAttribute<MessageViewer::MessageDisplayFormatAttribute>();
Akonadi::AttributeFactory::registerAttribute<MessageViewer::ScamAttribute>(); Akonadi::AttributeFactory::registerAttribute<MessageViewer::ScamAttribute>();
//Make sure to initialize it once.
if (messageviewer_initialize_database) {
WebEngineViewer::LocalDataBaseManager::self()->initialize();
}
} }
connect(WebEngineViewer::LocalDataBaseManager::self(), &WebEngineViewer::LocalDataBaseManager::checkUrlFinished, this, &ViewerPrivate::slotCheckedUrlFinished); mPhishingDatabase = new WebEngineViewer::LocalDataBaseManager(this);
mPhishingDatabase->initialize();
connect(mPhishingDatabase , &WebEngineViewer::LocalDataBaseManager::checkUrlFinished,
this, &ViewerPrivate::slotCheckedUrlFinished);
mShareServiceManager = new PimCommon::ShareServiceUrlManager(this); mShareServiceManager = new PimCommon::ShareServiceUrlManager(this);
...@@ -1987,7 +1985,7 @@ void ViewerPrivate::slotUrlOpen(const QUrl &url) ...@@ -1987,7 +1985,7 @@ void ViewerPrivate::slotUrlOpen(const QUrl &url)
void ViewerPrivate::checkPhishingUrl() void ViewerPrivate::checkPhishingUrl()
{ {
if (!PimCommon::NetworkUtil::self()->lowBandwidth() && MessageViewer::MessageViewerSettings::self()->checkPhishingUrl() && (mClickedUrl.scheme() != QLatin1String("mailto"))) { if (!PimCommon::NetworkUtil::self()->lowBandwidth() && MessageViewer::MessageViewerSettings::self()->checkPhishingUrl() && (mClickedUrl.scheme() != QLatin1String("mailto"))) {
WebEngineViewer::LocalDataBaseManager::self()->checkUrl(mClickedUrl); mPhishingDatabase->checkUrl(mClickedUrl);
} else { } else {
executeRunner(mClickedUrl); executeRunner(mClickedUrl);
} }
......
...@@ -79,6 +79,7 @@ namespace WebEngineViewer ...@@ -79,6 +79,7 @@ namespace WebEngineViewer
class WebHitTestResult; class WebHitTestResult;
class FindBarWebEngineView; class FindBarWebEngineView;
class ZoomActionMenu; class ZoomActionMenu;
class LocalDataBaseManager;
} }
namespace MessageViewer namespace MessageViewer
{ {
...@@ -694,6 +695,7 @@ public: ...@@ -694,6 +695,7 @@ public:
WebEngineViewer::ZoomActionMenu *mZoomActionMenu; WebEngineViewer::ZoomActionMenu *mZoomActionMenu;
QPrinter *mCurrentPrinter; QPrinter *mCurrentPrinter;
QList<QPointer<MessageViewer::MailSourceWebEngineViewer> > mListMailSourceViewer; QList<QPointer<MessageViewer::MailSourceWebEngineViewer> > mListMailSourceViewer;
WebEngineViewer::LocalDataBaseManager *mPhishingDatabase;
}; };
} }
......
...@@ -19,38 +19,48 @@ ...@@ -19,38 +19,48 @@
#include "localdatabasemanagertest.h" #include "localdatabasemanagertest.h"
#include "../localdatabasemanager.h" #include "../localdatabasemanager.h"
#include "../localdatabasemanager_p.h"
#include <QTest> #include <QTest>
class TestLocalDatabaseManagerPrivate : public WebEngineViewer::LocalDataBaseManagerPrivate
{
public:
TestLocalDatabaseManagerPrivate()
: WebEngineViewer::LocalDataBaseManagerPrivate()
{
}
protected:
void downloadDataBase(const QString &clientState) Q_DECL_OVERRIDE
{
// don't actually download anything
}
};
class TestLocalDataBaseManager : public WebEngineViewer::LocalDataBaseManager class TestLocalDataBaseManager : public WebEngineViewer::LocalDataBaseManager
{ {
public: public:
TestLocalDataBaseManager(QObject *parent) TestLocalDataBaseManager(QObject *parent)
: WebEngineViewer::LocalDataBaseManager(parent) : WebEngineViewer::LocalDataBaseManager(new TestLocalDatabaseManagerPrivate, parent)
{ {
} }
~TestLocalDataBaseManager()
{
delete d;
}
void setDownloadInfoSendByServer(const QString &data) void setDownloadInfoSendByServer(const QString &data)
{ {
mDownloadInfoSendByServer = data; mDownloadInfoSendByServer = data;
} }
// LocalDataBaseManager interface
protected:
void downloadFullDataBase() Q_DECL_OVERRIDE;
void downloadPartialDataBase() Q_DECL_OVERRIDE;
private: private:
QString mDownloadInfoSendByServer; QString mDownloadInfoSendByServer;
}; };
void TestLocalDataBaseManager::downloadFullDataBase()
{
}
void TestLocalDataBaseManager::downloadPartialDataBase()
{
}
LocalDataBaseManagerTest::LocalDataBaseManagerTest(QObject *parent) LocalDataBaseManagerTest::LocalDataBaseManagerTest(QObject *parent)
: QObject(parent) : QObject(parent)
......
...@@ -17,194 +17,48 @@ ...@@ -17,194 +17,48 @@
Boston, MA 02110-1301, USA. Boston, MA 02110-1301, USA.
*/ */
#include "localdatabasemanager.h" #include "localdatabasemanager.h"
#include "localdatabasemanager_p.h"
#include "webengineviewer_debug.h" #include "webengineviewer_debug.h"
#include "createphishingurldatabasejob.h" #include "createphishingurldatabasejob.h"
#include "createdatabasefilejob.h" #include "createdatabasefilejob.h"
#include "checkphishingurlutil.h" #include "checkphishingurlutil.h"
#include "localdatabasefile.h"
#include "downloadlocaldatabasethread.h"
#include "urlhashing.h" #include "urlhashing.h"
#include "backoffmodemanager.h" #include "backoffmodemanager.h"
#include <KConfigGroup> #include <KConfigGroup>
#include <KSharedConfig> #include <KSharedConfig>
#include <QPointer>
#include <QStandardPaths>
#include <QDebug>
#include <QDir>
#include <QTimer> #include <QTimer>
#include <QCryptographicHash> #include <QCryptographicHash>
using namespace WebEngineViewer; using namespace WebEngineViewer;
Q_GLOBAL_STATIC(LocalDataBaseManager, s_localDataBaseManager) Q_GLOBAL_STATIC(LocalDataBaseManagerPrivate, s_localDataBaseManager)
namespace LocalDataBaseManager::LocalDataBaseManager(LocalDataBaseManagerPrivate *impl, QObject *parent)
{
inline QString localDataBasePath()
{
return QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + QStringLiteral("/phishingurl/");
}
inline QString databaseFullPath()
{
return localDataBasePath() + QLatin1Char('/') + WebEngineViewer::CheckPhishingUrlUtil::databaseFileName();
}
}
class WebEngineViewer::LocalDataBaseManagerPrivate
{
public:
LocalDataBaseManagerPrivate(LocalDataBaseManager *qq)
: mFile(databaseFullPath()),
mSecondToStartRefreshing(0),
mDataBaseOk(false),
mDownloadProgress(false),
q(qq)
{
QDir().mkpath(localDataBasePath());
readConfig();
}
~LocalDataBaseManagerPrivate()
{
if (downloadLocalDatabaseThread) {
downloadLocalDatabaseThread->quit();
downloadLocalDatabaseThread->wait();
delete downloadLocalDatabaseThread;
}
saveConfig();
}
void readConfig();
void saveConfig();
LocalDataBaseFile mFile;
QString mNewClientState;
QString mMinimumWaitDuration;
uint mSecondToStartRefreshing;
bool mDataBaseOk;
bool mDownloadProgress;
QPointer<WebEngineViewer::DownloadLocalDatabaseThread> downloadLocalDatabaseThread;
LocalDataBaseManager *q;
};
LocalDataBaseManager::LocalDataBaseManager(QObject *parent)
: QObject(parent), : QObject(parent),
d(new LocalDataBaseManagerPrivate(this)) d(impl)
{ {
qRegisterMetaType<WebEngineViewer::UpdateDataBaseInfo>(); qRegisterMetaType<WebEngineViewer::UpdateDataBaseInfo>();
qRegisterMetaType<WebEngineViewer::CreatePhishingUrlDataBaseJob::DataBaseDownloadResult>(); qRegisterMetaType<WebEngineViewer::CreatePhishingUrlDataBaseJob::DataBaseDownloadResult>();
qRegisterMetaType<WebEngineViewer::CreatePhishingUrlDataBaseJob::ContraintsCompressionType>(); qRegisterMetaType<WebEngineViewer::CreatePhishingUrlDataBaseJob::ContraintsCompressionType>();
} }
LocalDataBaseManager::~LocalDataBaseManager() LocalDataBaseManager::LocalDataBaseManager(QObject *parent)
{ : LocalDataBaseManager(s_localDataBaseManager, parent)
delete d;
}
void LocalDataBaseManagerPrivate::readConfig()
{
KConfig phishingurlKConfig(WebEngineViewer::CheckPhishingUrlUtil::configFileName());
KConfigGroup grp = phishingurlKConfig.group(QStringLiteral("General"));
mNewClientState = grp.readEntry(QStringLiteral("DataBaseState"));
mMinimumWaitDuration = grp.readEntry(QStringLiteral("RefreshDataBase"));
if (!mMinimumWaitDuration.isEmpty()) {
mSecondToStartRefreshing = WebEngineViewer::CheckPhishingUrlUtil::refreshingCacheAfterThisTime(WebEngineViewer::CheckPhishingUrlUtil::convertToSecond(mMinimumWaitDuration));
}
}
void LocalDataBaseManagerPrivate::saveConfig()
{
KConfig phishingurlKConfig(WebEngineViewer::CheckPhishingUrlUtil::configFileName());
KConfigGroup grp = phishingurlKConfig.group(QStringLiteral("General"));
grp.writeEntry(QStringLiteral("DataBaseState"), mNewClientState);
grp.writeEntry(QStringLiteral("RefreshDataBase"), mMinimumWaitDuration);
}
void LocalDataBaseManager::downloadDataBase(const QString &clientState)
{
setDownloadProgress(true);
d->downloadLocalDatabaseThread = new WebEngineViewer::DownloadLocalDatabaseThread;
d->downloadLocalDatabaseThread->setDatabaseFullPath(databaseFullPath());
d->downloadLocalDatabaseThread->setDataBaseState(clientState);
connect(d->downloadLocalDatabaseThread.data(), &DownloadLocalDatabaseThread::createDataBaseFailed, this, &LocalDataBaseManager::slotCreateDataBaseFailed);
connect(d->downloadLocalDatabaseThread.data(), &DownloadLocalDatabaseThread::createDataBaseFinished, this, &LocalDataBaseManager::slotCreateDataBaseFileNameFinished);
connect(d->downloadLocalDatabaseThread.data(), &DownloadLocalDatabaseThread::finished, d->downloadLocalDatabaseThread.data(), &DownloadLocalDatabaseThread::deleteLater);
d->downloadLocalDatabaseThread->start();
}
void LocalDataBaseManager::slotCreateDataBaseFailed()
{ {
d->mDataBaseOk = false;
d->mDownloadProgress = false;
} }
void LocalDataBaseManager::downloadPartialDataBase()
{
downloadDataBase(d->mNewClientState);
}
void LocalDataBaseManager::downloadFullDataBase() LocalDataBaseManager::~LocalDataBaseManager()
{ {
downloadDataBase(QString());
} }
void LocalDataBaseManager::initialize() void LocalDataBaseManager::initialize()
{ {
if (d->mDownloadProgress) { d->initialize();
return;
}
if (!d->mDataBaseOk) {
qCDebug(WEBENGINEVIEWER_LOG) << "Start to create database";
if (!QFileInfo::exists(databaseFullPath())) {
downloadFullDataBase();
} else {
const uint now = QDateTime::currentDateTimeUtc().toTime_t();
//qDebug() << " now "<< now << " d->mSecondToStartRefreshing "<<d->mSecondToStartRefreshing << " now > d->mSecondToStartRefreshing" << (now > d->mSecondToStartRefreshing);
if ((d->mSecondToStartRefreshing != 0) && (d->mSecondToStartRefreshing > now)) {
qCWarning(WEBENGINEVIEWER_LOG) << " It's not necessary to check database now";
d->mDataBaseOk = true;
} else {
//Perhaps don't download for each start of kmail
downloadPartialDataBase();
}
}
} else {
qCWarning(WEBENGINEVIEWER_LOG) << "Database already initialized. It's a bug in code if you call it twice.";
}
}
void LocalDataBaseManager::slotCheckDataBase()
{
const uint now = QDateTime::currentDateTimeUtc().toTime_t();
if (d->mDataBaseOk && !d->mDownloadProgress && (d->mSecondToStartRefreshing < now)) {
downloadPartialDataBase();
}
}
void LocalDataBaseManager::slotCreateDataBaseFileNameFinished(bool success, const QString &newClientState, const QString &minimumWaitDurationStr)
{
d->mDataBaseOk = success;
d->mDownloadProgress = false;
d->mNewClientState = success ? newClientState : QString();
d->mMinimumWaitDuration = minimumWaitDurationStr;
d->saveConfig();
//if !success => redownload full!
if (!success) {
qCWarning(WEBENGINEVIEWER_LOG) << "We need to redownload full database";
downloadFullDataBase();
}
}
LocalDataBaseManager *LocalDataBaseManager::self()
{
return s_localDataBaseManager;
} }
void LocalDataBaseManager::setDownloadProgress(bool downloadProgress)
{
d->mDownloadProgress = downloadProgress;
}
void LocalDataBaseManager::checkUrl(const QUrl &url) void LocalDataBaseManager::checkUrl(const QUrl &url)
{ {
...@@ -235,7 +89,10 @@ void LocalDataBaseManager::checkUrl(const QUrl &url) ...@@ -235,7 +89,10 @@ void LocalDataBaseManager::checkUrl(const QUrl &url)
job->setDatabaseState(QStringList() << d->mNewClientState); job->setDatabaseState(QStringList() << d->mNewClientState);
job->setSearchHashs(conflictHashs); job->setSearchHashs(conflictHashs);
job->setSearchFullHashForUrl(url); job->setSearchFullHashForUrl(url);
connect(job, &SearchFullHashJob::result, this, &LocalDataBaseManager::slotSearchOnServerResult); connect(job, &SearchFullHashJob::result,
this, [this](CheckPhishingUrlUtil::UrlStatus status, const QUrl &url) {
Q_EMIT checkUrlFinished(url, status);
});
job->start(); job->start();
} }
} }
...@@ -247,9 +104,3 @@ void LocalDataBaseManager::checkUrl(const QUrl &url) ...@@ -247,9 +104,3 @@ void LocalDataBaseManager::checkUrl(const QUrl &url)
d->mFile.reload(); d->mFile.reload();
} }
} }
void LocalDataBaseManager::slotSearchOnServerResult(WebEngineViewer::CheckPhishingUrlUtil::UrlStatus status, const QUrl &url)
{
qCWarning(WEBENGINEVIEWER_LOG) << " Url " << url << " status " << status;
Q_EMIT checkUrlFinished(url, status);
}
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include <QUrl> #include <QUrl>
namespace WebEngineViewer namespace WebEngineViewer
{ {
class WebEngineView;
class LocalDataBaseManagerPrivate; class LocalDataBaseManagerPrivate;
class WEBENGINEVIEWER_EXPORT LocalDataBaseManager : public QObject class WEBENGINEVIEWER_EXPORT LocalDataBaseManager : public QObject
{ {
...@@ -35,27 +36,16 @@ public: ...@@ -35,27 +36,16 @@ public:
explicit LocalDataBaseManager(QObject *parent = nullptr); explicit LocalDataBaseManager(QObject *parent = nullptr);
~LocalDataBaseManager(); ~LocalDataBaseManager();
static LocalDataBaseManager *self();
void checkUrl(const QUrl &url); void checkUrl(const QUrl &url);
void initialize(); void initialize();
void slotCreateDataBaseFileNameFinished(bool finished, const QString &newClientState, const QString &minimumWaitDurationStr);
Q_SIGNALS: Q_SIGNALS:
void checkUrlFinished(const QUrl &url, WebEngineViewer::CheckPhishingUrlUtil::UrlStatus status); void checkUrlFinished(const QUrl &url, WebEngineViewer::CheckPhishingUrlUtil::UrlStatus status);
protected: protected:
void setDownloadProgress(bool downloadProgress); explicit LocalDataBaseManager(LocalDataBaseManagerPrivate *impl,
virtual void downloadFullDataBase(); QObject *parent = nullptr);
virtual void downloadPartialDataBase();
private:
void slotSearchOnServerResult(WebEngineViewer::CheckPhishingUrlUtil::UrlStatus status, const QUrl &url);
void slotCheckDataBase();
void downloadDataBase(const QString &clientState);
void slotCreateDataBaseFailed();
LocalDataBaseManagerPrivate *const d; LocalDataBaseManagerPrivate *const d;
}; };
......
/*
Copyright (C) 2016-2017 Laurent Montel <montel@kde.org>
This library is free software; you can redistribute it and/or
modify it under the terms of the GNU Library General Public
License as published by the Free Software Foundation; either
version 2 of the License, or (at your option) any later version.
This library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Library General Public License for more details.
You should have received a copy of the GNU Library General Public License
along with this library; see the file COPYING.LIB. If not, write to
the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA.
*/
#include "localdatabasefile.h"
#include "webengineviewer_debug.h"
#include "downloadlocaldatabasethread.h"
#include <KConfigGroup>
#include <KConfig>
#include <QDir>
#include <QStandardPaths>
#include <QPointer>
namespace
{
inline QString localDataBasePath()
{
return QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation) + QStringLiteral("/phishingurl/");
}
inline QString databaseFullPath()
{
return localDataBasePath() + QLatin1Char('/') + WebEngineViewer::CheckPhishingUrlUtil::databaseFileName();
}
}
namespace WebEngineViewer
{
class LocalDataBaseManagerPrivate
{
public:
LocalDataBaseManagerPrivate()
: mFile(databaseFullPath()),
mSecondToStartRefreshing(0),
mDataBaseOk(false),
mDownloadProgress(false)
{
QDir().mkpath(localDataBasePath());
readConfig();
}
virtual ~LocalDataBaseManagerPrivate()
{
if (downloadLocalDatabaseThread) {
downloadLocalDatabaseThread->quit();
downloadLocalDatabaseThread->wait();
delete downloadLocalDatabaseThread;
}
saveConfig();
}
void initialize()
{
if (mDownloadProgress) {
return;
}
if (!mDataBaseOk) {
qCDebug(WEBENGINEVIEWER_LOG) << "Start to create database";
if (!QFileInfo::exists(databaseFullPath())) {
downloadDataBase(QString());
} else {
const uint now = QDateTime::currentDateTimeUtc().toTime_t();
//qDebug() << " now "<< now << " d->mSecondToStartRefreshing "<<d->mSecondToStartRefreshing << " now > d->mSecondToStartRefreshing" << (now > d->mSecondToStartRefreshing);
if ((mSecondToStartRefreshing != 0) && (mSecondToStartRefreshing > now)) {
qCWarning(WEBENGINEVIEWER_LOG) << " It's not necessary to check database now";
mDataBaseOk = true;
} else {
//Perhaps don't download for each start of kmail