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

5 files changed, 141 insertions, 18 deletions

diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt
index e5d3d6468..3f906d187 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 a7f3cd4dc..8938e6082 100644
--- a/src/tests/testdir.cpp
+++ b/src/tests/testdir.cpp
@@ -27,7 +27,8 @@
 #include <sys/utime.h>
 #endif
 
-TestDir::TestDir()
+TestDir::TestDir(const QString& directoryPrefix) :
+    KTempDir(directoryPrefix)
 {
 }
 
diff --git a/src/tests/testdir.h b/src/tests/testdir.h
index 80e519e6b..0d3c5dd8d 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 000000000..c459f6874
--- /dev/null
+++ b/src/tests/viewpropertiestest.cpp
@@ -0,0 +1,101 @@
+/***************************************************************************
+ *   Copyright (C) 2012 by Peter Penz <[email protected]>             *
+ *                                                                         *
+ *   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 <qtest_kde.h>
+
+#include "dolphin_generalsettings.h"
+#include "views/viewproperties.h"
+#include "testdir.h"
+
+#include <QDebug>
+#include <QDir>
+
+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 125ac749e..96a00dc1d 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<ViewProperties*>(this)->convertNameRoleToTextRole();
-    }
-
     return m_node->sortRole().toLatin1();
 }
 
@@ -215,6 +227,10 @@ bool ViewProperties::sortFoldersFirst() const
 
 void ViewProperties::setVisibleRoles(const QList<QByteArray>& roles)
 {
+    if (roles == visibleRoles()) {
+        return;
+    }
+
     // See ViewProperties::visibleRoles() for the storage format
     // of the additional information.
 
@@ -272,18 +288,7 @@ QList<QByteArray> 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<ViewProperties*>(this)->convertAdditionalInfo();
-        visibleRoles = m_node->visibleRoles();
-    } else if (version <= NameRolePropertiesVersion) {
-        const_cast<ViewProperties*>(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();
 }