Merge lp:~3v1n0/u1db-qt/uri-path-parsing into lp:u1db-qt

[Marco Trevisan (Treviño)]

Database: support parsing of URI paths using something such as "file://"+ path

Make the database to parse and use local URIs starting with file://

Unfortunately this does not fix lp:1426178, which seems related to something
else.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:124
http://jenkins.qa.ubuntu.com/job/u1db-qt-ci/61/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-vivid-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-vivid-armhf-ci/8

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/u1db-qt-ci/61/rebuild

review: Approve (continuous-integration)

[Christian Dywan (kalikiana)]

It seems unexpected to me that passing a URI would set the property to a different value - an API consumer should be able to assume that you can get back what you set. So initializeIfNeeded should be where the URI is supported, which is also where :memory: is special-cased. And maybe in the unit test we can check that there's no error in addition to comparing the path?

review: Needs Fixing

[lp:~3v1n0/u1db-qt/uri-path-parsing]

125. By Marco Trevisan (Treviño) on 2015-03-05

Database: don't change the internal m_path value when sanitizing, just keep the user value

And sanitize it when creating the Db

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:127
http://jenkins.qa.ubuntu.com/job/u1db-qt-ci/64/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/u1db-qt-vivid-amd64-ci/11/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/u1db-qt-vivid-armhf-ci/11/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/u1db-qt-ci/64/rebuild

review: Needs Fixing (continuous-integration)

[Christian Dywan (kalikiana)]

You left out the bit where the folder is being created. That is needed.

review: Needs Fixing

[lp:~3v1n0/u1db-qt/uri-path-parsing]

126. By Marco Trevisan (Treviño) on 2015-03-05

TestDatabase: open temporary files, to actually create paths

And update QUrl test

[Christian Dywan (kalikiana)]

I didn't realize how the change of the if/else in fact deduplicate code here, so in fact it's all good now and a nice clean-up on top of it! Thanks a lot!

review: Approve

[lp:~3v1n0/u1db-qt/uri-path-parsing]

127. By Marco Trevisan (Treviño) on 2015-03-05

Database: keep the ":memory:" special path saved in Database::MEMORY_PATH

128. By Marco Trevisan (Treviño) on 2015-03-05

Database: make sure that setting an empty path, moves the db to memory

[Marco Trevisan (Treviño) (3v1n0)]

Cool... On a side: is there any reason why the path is not set as ":memory:" by default (but as an empty string)?

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:126
http://jenkins.qa.ubuntu.com/job/u1db-qt-ci/65/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-vivid-amd64-ci/12
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-vivid-armhf-ci/12

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/u1db-qt-ci/65/rebuild

review: Approve (continuous-integration)

[Christian Dywan (kalikiana)]

The original reasoning is this: If no path is set the database is in memory. Unlike the Python implementation the code path is exactly the same as using a file, relying on sqlite's support for :memory:. Only the explicit support for relative paths and now URIs requires code that is aware of the :memory: case. API users don't have to know about it either.

=== modified file 'examples/u1db-qt-example-1/u1db-qt-example-1.qml'
--- examples/u1db-qt-example-1/u1db-qt-example-1.qml	2013-05-02 18:20:33 +0000
+++ examples/u1db-qt-example-1/u1db-qt-example-1.qml	2015-03-05 17:51:40 +0000
@@ -36,7 +36,7 @@
     
     U1db.Database {
         id: aDatabase
-        path: "aDatabase1"
+        path: "file:///tmp/aDatabase1.db";
     }
     
     /*!

=== modified file 'src/database.cpp'
--- src/database.cpp	2015-02-19 10:37:54 +0000
+++ src/database.cpp	2015-03-05 17:51:40 +0000
@@ -24,6 +24,7 @@
 #include <QStandardPaths>
 #include <QDir>
 #include <QSqlError>
+#include <QUrl>
 #include <QUuid>
 #include <QStringList>
 #include <QJsonDocument>
@@ -34,6 +35,8 @@
 
 QT_BEGIN_NAMESPACE_U1DB
 
+const QString Database::MEMORY_PATH = ":memory:";
+
 namespace
 {
 class ScopedTransaction
@@ -85,6 +88,32 @@
 }
 
 /*!
+    Sanitize path
+ */
+QString Database::sanitizePath(const QString& path)
+{
+    if (path == Database::MEMORY_PATH)
+        return path;
+
+    if (!path.count())
+        return Database::MEMORY_PATH;
+
+    QUrl url(path);
+
+    if (url.isValid() && url.isLocalFile())
+    {
+        return url.path();
+    }
+    else if (QDir::isRelativePath(path))
+    {
+        QString dataPath(QStandardPaths::writableLocation(QStandardPaths::DataLocation));
+        return QDir(dataPath).absoluteFilePath(path);
+    }
+
+    return path;
+}
+
+/*!
     Checks if the underlying SQLite database is ready to be used
     Only to be used as a utility function by initializeIfNeeded()
  */
@@ -140,24 +169,17 @@
     if (!m_db.isValid())
         return setError("QSqlDatabase error");
 
-    if (path != ":memory:" && QDir::isRelativePath(path)) {
-        QString dataPath(QStandardPaths::writableLocation(QStandardPaths::DataLocation));
-        QString absolutePath(QDir(dataPath).absoluteFilePath(path));
-        QString parent(QFileInfo(absolutePath).dir().path());
-        if (!QDir().mkpath(parent))
-            setError(QString("Failed to make data folder %1").arg(parent));
-        m_db.setDatabaseName(absolutePath);
-    }
-    else
+    if (path != Database::MEMORY_PATH)
     {
         QDir parent(QFileInfo(path).dir());
         if (!parent.mkpath(parent.path()))
             setError(QString("Failed to make parent folder %1").arg(parent.path()));
-        m_db.setDatabaseName(path);
     }
 
+    m_db.setDatabaseName(path);
+
     if (!m_db.open())
-        return setError(QString("Failed to open %1: %2").arg(path).arg(m_db.lastError().text()));
+        return setError(QString("Failed to open '%1`: %2").arg(path).arg(m_db.lastError().text()));
     if (!isInitialized())
     {
         if (!isInitialized())
@@ -751,11 +773,11 @@
 
     beginResetModel();
     m_db.close();
-    initializeIfNeeded(path);
+    initializeIfNeeded(sanitizePath(path));
     endResetModel();
 
     m_path = path;
-    Q_EMIT pathChanged(path);
+    Q_EMIT pathChanged(m_path);
 }
 
 /*!

=== modified file 'src/database.h'
--- src/database.h	2014-01-24 11:13:35 +0000
+++ src/database.h	2015-03-05 17:51:40 +0000
@@ -77,13 +77,16 @@
     void docLoaded(const QString& docId, QVariant content) const;
 private:
     //Q_DISABLE_COPY(Database)
+    static const QString MEMORY_PATH;
+
     QString m_path;
     QSqlDatabase m_db;
     QString m_error;
 
     QString getReplicaUid();
+    QString sanitizePath(const QString& path);
     bool isInitialized();
-    bool initializeIfNeeded(const QString& path=":memory:");
+    bool initializeIfNeeded(const QString& path=Database::MEMORY_PATH);
     bool setError(const QString& error);
     QString getDocIdByRow(int row) const;
 

=== modified file 'tests/test-database.cpp'
--- tests/test-database.cpp	2014-11-07 14:30:31 +0000
+++ tests/test-database.cpp	2015-03-05 17:51:40 +0000
@@ -43,8 +43,36 @@
         QCOMPARE(db.getPath(), QString(""));
         QSignalSpy modelReset(&db, SIGNAL(pathChanged(const QString&)));
         QTemporaryFile file;
-        db.setPath(file.fileName());
-        QCOMPARE(db.getPath(), file.fileName());
+        QCOMPARE(file.open(), true);
+        db.setPath(file.fileName());
+        QCOMPARE(db.getPath(), file.fileName());
+        QVERIFY(db.lastError().isEmpty());
+    }
+
+    void testCanSetEmptyPath()
+    {
+        Database db;
+        QCOMPARE(db.getPath(), QString());
+        QSignalSpy modelReset(&db, SIGNAL(pathChanged(const QString&)));
+        QTemporaryFile file;
+        QCOMPARE(file.open(), true);
+        db.setPath(file.fileName());
+        QCOMPARE(db.getPath(), file.fileName());
+        db.setPath("");
+        QCOMPARE(db.getPath(), QString());
+        QVERIFY(db.lastError().isEmpty());
+    }
+
+    void testCanSetPathUsingQUrl()
+    {
+        Database db;
+        QCOMPARE(db.getPath(), QString(""));
+        QSignalSpy modelReset(&db, SIGNAL(pathChanged(const QString&)));
+        QTemporaryFile file;
+        QCOMPARE(file.open(), true);
+        QString url = QUrl::fromLocalFile(file.fileName()).toString();
+        db.setPath(url);
+        QCOMPARE(db.getPath(), url);
     }
 
     void testNonExistingParentFolder()