U1DB Qt/ QML

Merge lp://qastaging/~3v1n0/u1db-qt/uri-path-parsing into lp://qastaging/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://qastaging/~3v1n0/u1db-qt/uri-path-parsing
Merge into: lp://qastaging/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://qastaging/~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.qastaging.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://qastaging/~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://qastaging/~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://qastaging/~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
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: