refactor SettingsStorage class

1. Extract "transaction" support for QSettings into separate class
TransactionalSettings.
2. Define macrto with explicit name for the case when this "transaction"
support is needed.
3. A bit optimize QHash <-> QSettings copying: replace assign with
insert() and remove repetitive key lookups.
4. In save() check dirty status before getting the lock too.

The changes from items 1 and 2 make text more structured and the logic
of the SettingsStorage class gets separated from the implementation level
task of guarding the settings serialization. The changes in 3 and 4 do not
make the app much faster, but neither make any harm to the code readability.
This commit is contained in:
Eugene Shalygin 2016-03-15 06:09:17 +01:00
parent cd2496215e
commit cc09e7e834

View file

@ -27,6 +27,9 @@
* exception statement from your version. * exception statement from your version.
*/ */
#include "settingsstorage.h"
#include <memory>
#include <QFile> #include <QFile>
#include <QHash> #include <QHash>
#include <QStringList> #include <QStringList>
@ -34,18 +37,47 @@
#include "logger.h" #include "logger.h"
#include "utils/fs.h" #include "utils/fs.h"
#include "settingsstorage.h"
#ifdef Q_OS_MAC
#define QSETTINGS_SYNC_IS_SAVE // whether QSettings::sync() is "atomic"
#endif
namespace namespace
{ {
inline QSettings *createSettings(const QString &name) // Encapsulates serialization of settings in "atomic" way.
// write() does not leave half-written files,
// read() has a workaround for a case of power loss during a previous serialization
class TransactionalSettings
{ {
public:
TransactionalSettings(const QString &name)
: m_name(name)
{
}
QVariantHash read();
bool write(const QVariantHash &data);
private:
// we return actual file names used by QSettings because
// there is no other way to get that name except
// actually create a QSettings object.
// if serialization operation was not successful we return empty string
QString deserialize(const QString &name, QVariantHash &data);
QString serialize(const QString &name, const QVariantHash &data);
using SettingsPtr = std::unique_ptr<QSettings>;
SettingsPtr createSettings(const QString &name)
{
#ifdef Q_OS_WIN #ifdef Q_OS_WIN
return new QSettings(QSettings::IniFormat, QSettings::UserScope, "qBittorrent", name); return SettingsPtr(new QSettings(QSettings::IniFormat, QSettings::UserScope, "qBittorrent", name));
#else #else
return new QSettings("qBittorrent", name); return SettingsPtr(new QSettings("qBittorrent", name));
#endif #endif
} }
QString m_name;
};
#ifdef QBT_USES_QT5 #ifdef QBT_USES_QT5
typedef QHash<QString, QString> MappingTable; typedef QHash<QString, QString> MappingTable;
@ -93,48 +125,10 @@ namespace
SettingsStorage *SettingsStorage::m_instance = nullptr; SettingsStorage *SettingsStorage::m_instance = nullptr;
SettingsStorage::SettingsStorage() SettingsStorage::SettingsStorage()
: m_dirty(false) : m_data{TransactionalSettings(QLatin1String("qBittorrent")).read()}
, m_dirty(false)
, m_lock(QReadWriteLock::Recursive) , m_lock(QReadWriteLock::Recursive)
{ {
QSettings *settings;
#ifdef Q_OS_MAC
settings = createSettings("qBittorrent");
#else
settings = createSettings("qBittorrent_new");
QString newPath = settings->fileName();
// This means that the PC closed either due to power outage
// or because the disk was full. In any case the settings weren't transfered
// in their final position. So assume that qbittorrent_new.ini/qbittorrent_new.conf
// contains the most recent settings.
if (!settings->allKeys().isEmpty()) {
Logger::instance()->addMessage(tr("Detected unclean program exit. Using fallback file to restore settings."), Log::WARNING);
m_dirty = true;
}
else {
delete settings;
settings = createSettings("qBittorrent");
}
#endif
QStringList keys = settings->allKeys();
// Copy everything into memory. This means even keys inserted in the file manually
// or that we don't touch directly in this code(eg disabled by ifdef). This ensures
// that they will be copied over when save our settings to disk.
foreach (const QString &key, keys)
m_data[key] = settings->value(key);
//Ensures sync to disk before we attempt to manipulate the files from save().
delete settings;
#ifndef Q_OS_MAC
Utils::Fs::forceRemove(newPath);
if (m_dirty)
save();
#endif
m_timer.setSingleShot(true); m_timer.setSingleShot(true);
m_timer.setInterval(5 * 1000); m_timer.setInterval(5 * 1000);
connect(&m_timer, SIGNAL(timeout()), SLOT(save())); connect(&m_timer, SIGNAL(timeout()), SLOT(save()));
@ -164,54 +158,17 @@ SettingsStorage *SettingsStorage::instance()
bool SettingsStorage::save() bool SettingsStorage::save()
{ {
if (!m_dirty) return false; // Obtaining the lock is expensive, let's check early
QWriteLocker locker(&m_lock); QWriteLocker locker(&m_lock);
if (!m_dirty) return false; // something might have changed while we were getting the lock
if (!m_dirty) return false; TransactionalSettings settings(QLatin1String("qBittorrent"));
if (settings.write(m_data)) {
#ifndef Q_OS_MAC m_dirty = false;
// QSettings delete the file before writing it out. This can result in problems return true;
// if the disk is full or a power outage occurs. Those events might occur
// between deleting the file and recreating it. This is a safety measure.
// Write everything to qBittorrent_new.ini/qBittorrent_new.conf and if it succeeds
// replace qBittorrent_new.ini/qBittorrent.conf with it.
QSettings *settings = createSettings("qBittorrent_new");
#else
QSettings *settings = createSettings("qBittorrent");
#endif
foreach (const QString &key, m_data.keys())
settings->setValue(key, m_data[key]);
m_dirty = false;
locker.unlock();
#ifndef Q_OS_MAC
settings->sync(); // Important to get error status
QSettings::Status status = settings->status();
QString newPath = settings->fileName();
if (status != QSettings::NoError) {
if (status == QSettings::AccessError)
Logger::instance()->addMessage(tr("An access error occurred while trying to write the configuration file."), Log::CRITICAL);
else
Logger::instance()->addMessage(tr("A format error occurred while trying to write the configuration file."), Log::CRITICAL);
delete settings;
Utils::Fs::forceRemove(newPath);
return false;
} }
delete settings; return false;
QString finalPath = newPath;
int index = finalPath.lastIndexOf("_new", -1, Qt::CaseInsensitive);
finalPath.remove(index, 4);
Utils::Fs::forceRemove(finalPath);
QFile::rename(newPath, finalPath);
#else
delete settings;
#endif
return true;
} }
QVariant SettingsStorage::loadValue(const QString &key, const QVariant &defaultValue) const QVariant SettingsStorage::loadValue(const QString &key, const QVariant &defaultValue) const
@ -226,8 +183,8 @@ void SettingsStorage::storeValue(const QString &key, const QVariant &value)
QWriteLocker locker(&m_lock); QWriteLocker locker(&m_lock);
if (m_data.value(realKey) != value) { if (m_data.value(realKey) != value) {
m_dirty = true; m_dirty = true;
m_timer.start();
m_data.insert(realKey, value); m_data.insert(realKey, value);
m_timer.start();
} }
} }
@ -237,7 +194,94 @@ void SettingsStorage::removeValue(const QString &key)
QWriteLocker locker(&m_lock); QWriteLocker locker(&m_lock);
if (m_data.contains(realKey)) { if (m_data.contains(realKey)) {
m_dirty = true; m_dirty = true;
m_timer.start();
m_data.remove(realKey); m_data.remove(realKey);
m_timer.start();
} }
} }
QVariantHash TransactionalSettings::read()
{
QVariantHash res;
#ifdef QSETTINGS_SYNC_IS_SAVE
deserialize(m_name, res);
#else
bool writeBackNeeded = false;
QString newPath = deserialize(m_name + QLatin1String("_new"), res);
if (!newPath.isEmpty()) { // "_new" file is NOT empty
// This means that the PC closed either due to power outage
// or because the disk was full. In any case the settings weren't transfered
// in their final position. So assume that qbittorrent_new.ini/qbittorrent_new.conf
// contains the most recent settings.
Logger::instance()->addMessage(QObject::tr("Detected unclean program exit. Using fallback file to restore settings."), Log::WARNING);
writeBackNeeded = true;
}
else {
deserialize(m_name, res);
}
Utils::Fs::forceRemove(newPath);
if (writeBackNeeded)
write(res);
#endif
return res;
}
bool TransactionalSettings::write(const QVariantHash &data)
{
#ifdef QSETTINGS_SYNC_IS_SAVE
serialize(m_name, data);
#else
// QSettings delete the file before writing it out. This can result in problems
// if the disk is full or a power outage occurs. Those events might occur
// between deleting the file and recreating it. This is a safety measure.
// Write everything to qBittorrent_new.ini/qBittorrent_new.conf and if it succeeds
// replace qBittorrent.ini/qBittorrent.conf with it.
QString newPath = serialize(m_name + QLatin1String("_new"), data);
if (newPath.isEmpty()) {
Utils::Fs::forceRemove(newPath);
return false;
}
QString finalPath = newPath;
int index = finalPath.lastIndexOf("_new", -1, Qt::CaseInsensitive);
finalPath.remove(index, 4);
Utils::Fs::forceRemove(finalPath);
QFile::rename(newPath, finalPath);
#endif
return true;
}
QString TransactionalSettings::deserialize(const QString &name, QVariantHash &data)
{
SettingsPtr settings = createSettings(name);
if (settings->allKeys().isEmpty())
return QString();
// Copy everything into memory. This means even keys inserted in the file manually
// or that we don't touch directly in this code (eg disabled by ifdef). This ensures
// that they will be copied over when save our settings to disk.
foreach (const QString &key, settings->allKeys())
data.insert(key, settings->value(key));
return settings->fileName();
}
QString TransactionalSettings::serialize(const QString &name, const QVariantHash &data)
{
SettingsPtr settings = createSettings(name);
for (auto i = data.begin(); i != data.end(); ++i)
settings->setValue(i.key(), i.value());
settings->sync(); // Important to get error status
QSettings::Status status = settings->status();
if (status != QSettings::NoError) {
if (status == QSettings::AccessError)
Logger::instance()->addMessage(QObject::tr("An access error occurred while trying to write the configuration file."), Log::CRITICAL);
else
Logger::instance()->addMessage(QObject::tr("A format error occurred while trying to write the configuration file."), Log::CRITICAL);
return QString();
}
return settings->fileName();
}