Merge lp:~gcollura/u1db-qt/fix-1387294 into lp:u1db-qt

Proposed by Giulio Collura on 2014-10-29
Status: Needs review
Proposed branch: lp:~gcollura/u1db-qt/fix-1387294
Merge into: lp:u1db-qt
Diff against target: 32 lines (+12/-9)
1 file modified
src/database.cpp (+12/-9)
To merge this branch: bzr merge lp:~gcollura/u1db-qt/fix-1387294
Reviewer Review Type Date Requested Status
Michał Karnicki Needs Information on 2015-03-18
Christian Dywan 2014-10-29 Needs Information on 2014-10-30
Review via email: mp+240035@code.launchpad.net

Commit message

Make u1db-qt play nice with ~ as absolute directory.

Description of the change

Fix for bug #1387294. Make u1db-qt play nice with ~ as absolute directory.

To post a comment you must log in.
lp:~gcollura/u1db-qt/fix-1387294 updated on 2014-10-29
121. By Giulio Collura on 2014-10-29

improve codestyle

Michał Karnicki (karni) wrote :

Looks good to me. Can we please have someone from U1DB Qt devs review and advise on how we get this landed? Thanks.

Christian Dywan (kalikiana) wrote :

My immediate concern is that this isn't addressing the use case it was motivated by - if settings stored in U1Db should go to the config folder, ~ is the wrong approach.

review: Needs Information
Giulio Collura (gcollura) wrote :

What would you suggest instead? We would like to have the chance to use .config for all data app. We can add another property to Database that stores the destination folder, for example:
Database {
  path: 'hello.db'
  location: Database.ConfigLocation
}
I'm not sure about the naming convention, the default option would be Database.DataLocation.
These would be values of an enum exported to qml.
Let me know what you think about this solution.

Giulio

Michał Karnicki (karni) wrote :

I have mixed feelings. Let's not forget U1DB can also be used by desktop apps, which currently are not confined and apparmor restrictions do not apply.

Michał Karnicki (karni) wrote :

Hi Christian,

Have you made decision how you want to proceed with this bug? Soon this will affect Telegram, where (IIUC) there no longer is 'phablet' user in the new image.

review: Needs Information

Unmerged revisions

121. By Giulio Collura on 2014-10-29

improve codestyle

120. By Giulio Collura on 2014-10-29

Fix bug #1387294. Improve database path creation.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/database.cpp'
2--- src/database.cpp 2014-07-09 22:39:59 +0000
3+++ src/database.cpp 2014-10-29 19:48:43 +0000
4@@ -140,16 +140,19 @@
5 if (!m_db.isValid())
6 return setError("QSqlDatabase error");
7
8- if (path != ":memory:" && QDir::isRelativePath(path)) {
9+ QString absolutePath;
10+ if (path != ":memory:" && QDir::isRelativePath(path) && !path.contains("~")) {
11 QString dataPath(QStandardPaths::writableLocation(QStandardPaths::DataLocation));
12- QString absolutePath(QDir(dataPath).absoluteFilePath(path));
13- QString parent(QFileInfo(absolutePath).dir().path());
14- if (!QDir().mkpath(parent))
15- qWarning() << "Failed to make data folder" << parent;
16- m_db.setDatabaseName(absolutePath);
17- }
18- else
19- m_db.setDatabaseName(path);
20+ absolutePath = QDir(dataPath).absoluteFilePath(path);
21+ }
22+ else {
23+ absolutePath = path;
24+ absolutePath.replace("~", QDir::homePath());
25+ }
26+ QString parent(QFileInfo(absolutePath).dir().path());
27+ if (!QDir().mkpath(parent))
28+ qWarning() << "Failed to make destination folder" << parent;
29+ m_db.setDatabaseName(absolutePath);
30
31 if (!m_db.open())
32 return setError(QString("Failed to open %1: %2").arg(path).arg(m_db.lastError().text()));

Subscribers

People subscribed via source and target branches

to all changes: