From d9eab08fcb8bfc56470a7f418465607888b5f1ba Mon Sep 17 00:00:00 2001 From: Peter Penz Date: Fri, 18 May 2012 19:39:03 +0200 Subject: [PATCH] Don't create a .directory-file for each directory This regression has been introduced on master and has not been released yet: Due to the changed properties-format an update has been done which resulted in writing a .directory file into each newly entered directory. The patch updates the view-properties and version only in the constructor so that it is assured that reading properties never accidently will change the internal version. A unit-test has been added to catch regressions like this in future. BUG: 300240 FIXED-IN: 4.9.0 --- src/tests/CMakeLists.txt | 12 ++++ src/tests/testdir.cpp | 3 +- src/tests/testdir.h | 2 +- src/tests/viewpropertiestest.cpp | 101 +++++++++++++++++++++++++++++++ src/views/viewproperties.cpp | 41 ++++++++----- 5 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 src/tests/viewpropertiestest.cpp diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index e5d3d6468a..3f906d1876 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -79,3 +79,15 @@ set(kstandarditemmodeltest_SRCS kde4_add_unit_test(kstandarditemmodeltest TEST ${kstandarditemmodeltest_SRCS}) target_link_libraries(kstandarditemmodeltest dolphinprivate ${KDE4_KIO_LIBS} ${QT_QTTEST_LIBRARY}) +# ViewPropertiesTest +set(viewpropertiestest_SRCS + viewpropertiestest.cpp + testdir.cpp + ../views/viewproperties.cpp +) +kde4_add_kcfg_files(viewpropertiestest_SRCS + ../settings/dolphin_generalsettings.kcfgc + ../settings/dolphin_directoryviewpropertysettings.kcfgc +) +kde4_add_unit_test(viewpropertiestest TEST ${viewpropertiestest_SRCS}) +target_link_libraries(viewpropertiestest dolphinprivate ${KDE4_KIO_LIBS} ${QT_QTTEST_LIBRARY}) diff --git a/src/tests/testdir.cpp b/src/tests/testdir.cpp index a7f3cd4dc4..8938e6082a 100644 --- a/src/tests/testdir.cpp +++ b/src/tests/testdir.cpp @@ -27,7 +27,8 @@ #include #endif -TestDir::TestDir() +TestDir::TestDir(const QString& directoryPrefix) : + KTempDir(directoryPrefix) { } diff --git a/src/tests/testdir.h b/src/tests/testdir.h index 80e519e6bc..0d3c5dd8d3 100644 --- a/src/tests/testdir.h +++ b/src/tests/testdir.h @@ -33,7 +33,7 @@ class TestDir : public KTempDir { public: - TestDir(); + TestDir(const QString& directoryPrefix = QString()); virtual ~TestDir(); KUrl url() const; diff --git a/src/tests/viewpropertiestest.cpp b/src/tests/viewpropertiestest.cpp new file mode 100644 index 0000000000..c459f68746 --- /dev/null +++ b/src/tests/viewpropertiestest.cpp @@ -0,0 +1,101 @@ +/*************************************************************************** + * Copyright (C) 2012 by Peter Penz * + * * + * This program is free software; you can redistribute it and/or modify * + * it under the terms of the GNU General Public License as published by * + * the Free Software Foundation; either version 2 of the License, or * + * (at your option) any later version. * + * * + * This program 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 General Public License for more details. * + * * + * You should have received a copy of the GNU General Public License * + * along with this program; if not, write to the * + * Free Software Foundation, Inc., * + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA * + ***************************************************************************/ + +#include + +#include "dolphin_generalsettings.h" +#include "views/viewproperties.h" +#include "testdir.h" + +#include +#include + +class ViewPropertiesTest : public QObject +{ + Q_OBJECT + +private slots: + void init(); + void cleanup(); + + void testReadOnlyBehavior(); + void testAutoSave(); + +private: + bool m_globalViewProps; + TestDir* m_testDir; +}; + +void ViewPropertiesTest::init() +{ + m_globalViewProps = GeneralSettings::self()->globalViewProps(); + GeneralSettings::self()->setGlobalViewProps(false); + + // It is mandatory to create the test-directory inside the home-directory + // of the user: ViewProperties does not write inside directories + // outside the home-directory to prevent overwriting other user-settings + // in case if write-permissions are given. + m_testDir = new TestDir(QDir::homePath() + "/.viewPropertiesTest-"); +} + +void ViewPropertiesTest::cleanup() +{ + delete m_testDir; + m_testDir = 0; + + GeneralSettings::self()->setGlobalViewProps(m_globalViewProps); +} + +/** + * Test whether only reading properties won't result in creating + * a .directory file when destructing the ViewProperties instance + * and autosaving is enabled. + */ +void ViewPropertiesTest::testReadOnlyBehavior() +{ + QString dotDirectoryFile = m_testDir->url().toLocalFile() + ".directory"; + QVERIFY(!QFile::exists(dotDirectoryFile)); + + ViewProperties* props = new ViewProperties(m_testDir->url()); + QVERIFY(props->isAutoSaveEnabled()); + const QByteArray sortRole = props->sortRole(); + Q_UNUSED(sortRole); + delete props; + props = 0; + + QVERIFY(!QFile::exists(dotDirectoryFile)); +} + +void ViewPropertiesTest::testAutoSave() +{ + QString dotDirectoryFile = m_testDir->url().toLocalFile() + ".directory"; + QVERIFY(!QFile::exists(dotDirectoryFile)); + + ViewProperties* props = new ViewProperties(m_testDir->url()); + QVERIFY(props->isAutoSaveEnabled()); + props->setSortRole("someNewSortRole"); + delete props; + props = 0; + + QVERIFY(QFile::exists(dotDirectoryFile)); +} + +QTEST_KDEMAIN(ViewPropertiesTest, NoGUI) + +#include "viewpropertiestest.moc" diff --git a/src/views/viewproperties.cpp b/src/views/viewproperties.cpp index 125ac749e0..96a00dc1dc 100644 --- a/src/views/viewproperties.cpp +++ b/src/views/viewproperties.cpp @@ -105,6 +105,22 @@ ViewProperties::ViewProperties(const KUrl& url) : m_changedProps = false; } } + + if (m_node->version() < CurrentViewPropertiesVersion) { + // The view-properties have an outdated version. Convert the properties + // to the changes of the current version. + if (m_node->version() < AdditionalInfoViewPropertiesVersion) { + convertAdditionalInfo(); + Q_ASSERT(m_node->version() == AdditionalInfoViewPropertiesVersion); + } + + if (m_node->version() < NameRolePropertiesVersion) { + convertNameRoleToTextRole(); + Q_ASSERT(m_node->version() == NameRolePropertiesVersion); + } + + m_node->setVersion(CurrentViewPropertiesVersion); + } } ViewProperties::~ViewProperties() @@ -180,10 +196,6 @@ void ViewProperties::setSortRole(const QByteArray& role) QByteArray ViewProperties::sortRole() const { - if (m_node->version() <= NameRolePropertiesVersion) { - const_cast(this)->convertNameRoleToTextRole(); - } - return m_node->sortRole().toLatin1(); } @@ -215,6 +227,10 @@ bool ViewProperties::sortFoldersFirst() const void ViewProperties::setVisibleRoles(const QList& roles) { + if (roles == visibleRoles()) { + return; + } + // See ViewProperties::visibleRoles() for the storage format // of the additional information. @@ -272,18 +288,7 @@ QList ViewProperties::visibleRoles() const const QString prefix = viewModePrefix(); const int prefixLength = prefix.length(); - QStringList visibleRoles = m_node->visibleRoles(); - const int version = m_node->version(); - if (visibleRoles.isEmpty() && version <= AdditionalInfoViewPropertiesVersion) { - // Convert the obsolete additionalInfo-property from older versions into the - // visibleRoles-property - const_cast(this)->convertAdditionalInfo(); - visibleRoles = m_node->visibleRoles(); - } else if (version <= NameRolePropertiesVersion) { - const_cast(this)->convertNameRoleToTextRole(); - visibleRoles = m_node->visibleRoles(); - } - + const QStringList visibleRoles = m_node->visibleRoles(); foreach (const QString& visibleRole, visibleRoles) { if (visibleRole.startsWith(prefix)) { const QByteArray role = visibleRole.right(visibleRole.length() - prefixLength).toLatin1(); @@ -330,6 +335,7 @@ void ViewProperties::setDirProperties(const ViewProperties& props) setSortFoldersFirst(props.sortFoldersFirst()); setVisibleRoles(props.visibleRoles()); setHeaderColumnWidths(props.headerColumnWidths()); + m_node->setVersion(props.m_node->version()); } void ViewProperties::setAutoSaveEnabled(bool autoSave) @@ -350,6 +356,7 @@ void ViewProperties::update() void ViewProperties::save() { + kDebug() << "Saving view-properties to" << m_filePath; KStandardDirs::makeDir(m_filePath); m_node->setVersion(CurrentViewPropertiesVersion); m_node->writeConfig(); @@ -411,6 +418,7 @@ void ViewProperties::convertAdditionalInfo() m_node->setAdditionalInfo(QStringList()); m_node->setVisibleRoles(visibleRoles); + m_node->setVersion(AdditionalInfoViewPropertiesVersion); update(); } @@ -431,6 +439,7 @@ void ViewProperties::convertNameRoleToTextRole() m_node->setVisibleRoles(visibleRoles); m_node->setSortRole(sortRole); + m_node->setVersion(NameRolePropertiesVersion); update(); }