U1DB Qt/ QML

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

  1. uri-path-parsing
  2. Merge into trunk
Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Cris Dywan
Approved revision: 126
Merged at revision: 128
Proposed branch: lp:~3v1n0/u1db-qt/uri-path-parsing
Merge into: lp:u1db-qt
Diff against target: 174 lines (+70/-17)
4 files modified
examples/u1db-qt-example-1/u1db-qt-example-1.qml (+1/-1)
src/database.cpp (+35/-13)
src/database.h (+4/-1)
tests/test-database.cpp (+30/-2)
To merge this branch: bzr merge lp:~3v1n0/u1db-qt/uri-path-parsing
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Cris Dywan Approve
Review via email: mp+251369@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Cris Dywan (kalikiana) wrote :

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 updated
125. By Marco Trevisan (Treviño)

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

And sanitize it when creating the Db

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Cris Dywan (kalikiana) wrote :

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

review: Needs Fixing
lp:~3v1n0/u1db-qt/uri-path-parsing updated
126. By Marco Trevisan (Treviño)

TestDatabase: open temporary files, to actually create paths

And update QUrl test

Revision history for this message
Cris Dywan (kalikiana) wrote :

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 updated
127. By Marco Trevisan (Treviño)

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

128. By Marco Trevisan (Treviño)

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

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

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

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Cris Dywan (kalikiana) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
36 36
37 U1db.Database {37 U1db.Database {
38 id: aDatabase38 id: aDatabase
39 path: "aDatabase1"39 path: "file:///tmp/aDatabase1.db";
40 }40 }
41 41
42 /*!42 /*!
4343
=== 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 @@
24#include <QStandardPaths>24#include <QStandardPaths>
25#include <QDir>25#include <QDir>
26#include <QSqlError>26#include <QSqlError>
27#include <QUrl>
27#include <QUuid>28#include <QUuid>
28#include <QStringList>29#include <QStringList>
29#include <QJsonDocument>30#include <QJsonDocument>
@@ -34,6 +35,8 @@
3435
35QT_BEGIN_NAMESPACE_U1DB36QT_BEGIN_NAMESPACE_U1DB
3637
38const QString Database::MEMORY_PATH = ":memory:";
39
37namespace40namespace
38{41{
39class ScopedTransaction42class ScopedTransaction
@@ -85,6 +88,32 @@
85}88}
8689
87/*!90/*!
91 Sanitize path
92 */
93QString Database::sanitizePath(const QString& path)
94{
95 if (path == Database::MEMORY_PATH)
96 return path;
97
98 if (!path.count())
99 return Database::MEMORY_PATH;
100
101 QUrl url(path);
102
103 if (url.isValid() && url.isLocalFile())
104 {
105 return url.path();
106 }
107 else if (QDir::isRelativePath(path))
108 {
109 QString dataPath(QStandardPaths::writableLocation(QStandardPaths::DataLocation));
110 return QDir(dataPath).absoluteFilePath(path);
111 }
112
113 return path;
114}
115
116/*!
88 Checks if the underlying SQLite database is ready to be used117 Checks if the underlying SQLite database is ready to be used
89 Only to be used as a utility function by initializeIfNeeded()118 Only to be used as a utility function by initializeIfNeeded()
90 */119 */
@@ -140,24 +169,17 @@
140 if (!m_db.isValid())169 if (!m_db.isValid())
141 return setError("QSqlDatabase error");170 return setError("QSqlDatabase error");
142171
143 if (path != ":memory:" && QDir::isRelativePath(path)) {172 if (path != Database::MEMORY_PATH)
144 QString dataPath(QStandardPaths::writableLocation(QStandardPaths::DataLocation));
145 QString absolutePath(QDir(dataPath).absoluteFilePath(path));
146 QString parent(QFileInfo(absolutePath).dir().path());
147 if (!QDir().mkpath(parent))
148 setError(QString("Failed to make data folder %1").arg(parent));
149 m_db.setDatabaseName(absolutePath);
150 }
151 else
152 {173 {
153 QDir parent(QFileInfo(path).dir());174 QDir parent(QFileInfo(path).dir());
154 if (!parent.mkpath(parent.path()))175 if (!parent.mkpath(parent.path()))
155 setError(QString("Failed to make parent folder %1").arg(parent.path()));176 setError(QString("Failed to make parent folder %1").arg(parent.path()));
156 m_db.setDatabaseName(path);
157 }177 }
158178
179 m_db.setDatabaseName(path);
180
159 if (!m_db.open())181 if (!m_db.open())
160 return setError(QString("Failed to open %1: %2").arg(path).arg(m_db.lastError().text()));182 return setError(QString("Failed to open '%1`: %2").arg(path).arg(m_db.lastError().text()));
161 if (!isInitialized())183 if (!isInitialized())
162 {184 {
163 if (!isInitialized())185 if (!isInitialized())
@@ -751,11 +773,11 @@
751773
752 beginResetModel();774 beginResetModel();
753 m_db.close();775 m_db.close();
754 initializeIfNeeded(path);776 initializeIfNeeded(sanitizePath(path));
755 endResetModel();777 endResetModel();
756778
757 m_path = path;779 m_path = path;
758 Q_EMIT pathChanged(path);780 Q_EMIT pathChanged(m_path);
759}781}
760782
761/*!783/*!
762784
=== 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 @@
77 void docLoaded(const QString& docId, QVariant content) const;77 void docLoaded(const QString& docId, QVariant content) const;
78private:78private:
79 //Q_DISABLE_COPY(Database)79 //Q_DISABLE_COPY(Database)
80 static const QString MEMORY_PATH;
81
80 QString m_path;82 QString m_path;
81 QSqlDatabase m_db;83 QSqlDatabase m_db;
82 QString m_error;84 QString m_error;
8385
84 QString getReplicaUid();86 QString getReplicaUid();
87 QString sanitizePath(const QString& path);
85 bool isInitialized();88 bool isInitialized();
86 bool initializeIfNeeded(const QString& path=":memory:");89 bool initializeIfNeeded(const QString& path=Database::MEMORY_PATH);
87 bool setError(const QString& error);90 bool setError(const QString& error);
88 QString getDocIdByRow(int row) const;91 QString getDocIdByRow(int row) const;
8992
9093
=== 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 @@
43 QCOMPARE(db.getPath(), QString(""));43 QCOMPARE(db.getPath(), QString(""));
44 QSignalSpy modelReset(&db, SIGNAL(pathChanged(const QString&)));44 QSignalSpy modelReset(&db, SIGNAL(pathChanged(const QString&)));
45 QTemporaryFile file;45 QTemporaryFile file;
46 db.setPath(file.fileName());46 QCOMPARE(file.open(), true);
47 QCOMPARE(db.getPath(), file.fileName());47 db.setPath(file.fileName());
48 QCOMPARE(db.getPath(), file.fileName());
49 QVERIFY(db.lastError().isEmpty());
50 }
51
52 void testCanSetEmptyPath()
53 {
54 Database db;
55 QCOMPARE(db.getPath(), QString());
56 QSignalSpy modelReset(&db, SIGNAL(pathChanged(const QString&)));
57 QTemporaryFile file;
58 QCOMPARE(file.open(), true);
59 db.setPath(file.fileName());
60 QCOMPARE(db.getPath(), file.fileName());
61 db.setPath("");
62 QCOMPARE(db.getPath(), QString());
63 QVERIFY(db.lastError().isEmpty());
64 }
65
66 void testCanSetPathUsingQUrl()
67 {
68 Database db;
69 QCOMPARE(db.getPath(), QString(""));
70 QSignalSpy modelReset(&db, SIGNAL(pathChanged(const QString&)));
71 QTemporaryFile file;
72 QCOMPARE(file.open(), true);
73 QString url = QUrl::fromLocalFile(file.fileName()).toString();
74 db.setPath(url);
75 QCOMPARE(db.getPath(), url);
48 }76 }
4977
50 void testNonExistingParentFolder()78 void testNonExistingParentFolder()

Subscribers

People subscribed via source and target branches

to all changes: