Merge lp:~jamesh/mediascanner2/bug-1501990 into lp:mediascanner2

Proposed by James Henstridge on 2015-10-07
Status: Merged
Approved by: Michi Henning on 2015-10-08
Approved revision: 309
Merged at revision: 310
Proposed branch: lp:~jamesh/mediascanner2/bug-1501990
Merge into: lp:mediascanner2
Diff against target: 98 lines (+34/-8)
2 files modified
src/mediascanner/MediaStore.cc (+7/-4)
test/test_mediastore.cc (+27/-4)
To merge this branch: bzr merge lp:~jamesh/mediascanner2/bug-1501990
Reviewer Review Type Date Requested Status
Michi Henning (community) 2015-10-07 Approve on 2015-10-08
Review via email: mp+273649@code.launchpad.net

Commit message

Remove the id column from media_attic so that we don't get conflicts when copying data between media and media_attic.

Description of the change

When removable media is plugged or unplugged, the media scanner copies rows from the media table to or from the media_attic table (the idea being to avoid the need to keep the metadata from the removable media around while it is unplugged but not in the search results).

In 0.106, we added an explicit integer primary key field to media and media_attic, since previously it was using the implicit rowid as a foreign key in the FTS index, which is not guaranteed to stay unchanged over various operations (e.g. vacuum). However, it seems the sequence used to allocate these IDs could be reuse values when they were removed from the table leading to duplicates. This meant that a future copy operation could fail.

Since nothing references the media_attic table, the easy solution was to simply remove its id column so IDs will be stripped when copying data to the attic, and new IDs assigned when copying back to the media table.

To post a comment you must log in.
Michi Henning (michihenning) wrote :

Looks good!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/mediascanner/MediaStore.cc'
2--- src/mediascanner/MediaStore.cc 2015-07-14 01:34:56 +0000
3+++ src/mediascanner/MediaStore.cc 2015-10-07 07:13:13 +0000
4@@ -46,7 +46,7 @@
5
6 // Increment this whenever changing db schema.
7 // It will cause dbstore to rebuild its tables.
8-static const int schemaVersion = 9;
9+static const int schemaVersion = 10;
10
11 struct MediaStorePrivate {
12 sqlite3 *db;
13@@ -322,7 +322,6 @@
14 CREATE INDEX media_mtime_idx ON media(type, mtime);
15
16 CREATE TABLE media_attic (
17- id INTEGER PRIMARY KEY,
18 filename TEXT UNIQUE NOT NULL,
19 content_type TEXT,
20 etag TEXT,
21@@ -918,7 +917,9 @@
22
23 void MediaStorePrivate::archiveItems(const std::string &prefix) {
24 const char *templ = R"(BEGIN TRANSACTION;
25-INSERT INTO media_attic SELECT * FROM media WHERE filename LIKE %s;
26+INSERT INTO media_attic (filename, content_type, etag, title, date, artist, album, album_artist, genre, disc_number, track_number, duration, width, height, latitude, longitude, has_thumbnail, mtime, type)
27+ SELECT filename, content_type, etag, title, date, artist, album, album_artist, genre, disc_number, track_number, duration, width, height, latitude, longitude, has_thumbnail, mtime, type
28+ FROM media WHERE filename LIKE %s;
29 DELETE FROM media WHERE filename LIKE %s;
30 COMMIT;
31 )";
32@@ -935,7 +936,9 @@
33
34 void MediaStorePrivate::restoreItems(const std::string &prefix) {
35 const char *templ = R"(BEGIN TRANSACTION;
36-INSERT INTO media SELECT * FROM media_attic WHERE filename LIKE %s;
37+INSERT INTO media (filename, content_type, etag, title, date, artist, album, album_artist, genre, disc_number, track_number, duration, width, height, latitude, longitude, has_thumbnail, mtime, type)
38+ SELECT filename, content_type, etag, title, date, artist, album, album_artist, genre, disc_number, track_number, duration, width, height, latitude, longitude, has_thumbnail, mtime, type
39+ FROM media_attic WHERE filename LIKE %s;
40 DELETE FROM media_attic WHERE filename LIKE %s;
41 COMMIT;
42 )";
43
44=== modified file 'test/test_mediastore.cc'
45--- test/test_mediastore.cc 2015-07-14 01:34:56 +0000
46+++ test/test_mediastore.cc 2015-10-07 07:13:13 +0000
47@@ -24,9 +24,10 @@
48 #include <mediascanner/MediaStore.hh>
49 #include <mediascanner/internal/utils.hh>
50
51-#include<stdexcept>
52-#include<cstdio>
53-#include<string>
54+#include <algorithm>
55+#include <stdexcept>
56+#include <cstdio>
57+#include <string>
58 #include <gtest/gtest.h>
59
60 using namespace std;
61@@ -488,7 +489,23 @@
62 TEST_F(MediaStoreTest, unmount) {
63 MediaFile audio1 = MediaFileBuilder("/media/username/dir/fname.ogg")
64 .setType(AudioMedia)
65- .setTitle("bbb bbb");
66+ .setETag("etag")
67+ .setContentType("audio/ogg")
68+ .setTitle("bbb bbb")
69+ .setDate("2015-01-01")
70+ .setAuthor("artist")
71+ .setAlbum("album")
72+ .setAlbumArtist("album_artist")
73+ .setGenre("genre")
74+ .setDiscNumber(5)
75+ .setTrackNumber(10)
76+ .setDuration(42)
77+ .setWidth(640)
78+ .setHeight(480)
79+ .setLatitude(8.0)
80+ .setLongitude(4.0)
81+ .setHasThumbnail(true)
82+ .setModificationTime(10000);
83 MediaFile audio2 = MediaFileBuilder("/home/username/Music/fname.ogg")
84 .setType(AudioMedia)
85 .setTitle("bbb bbb");
86@@ -507,6 +524,12 @@
87 store.restoreItems("/media/username");
88 result = store.query("bbb", AudioMedia, filter);
89 ASSERT_EQ(result.size(), 2);
90+ std::sort(result.begin(), result.end(),
91+ [](const MediaFile &m1, const MediaFile &m2) -> bool {
92+ return m1.getFileName() < m2.getFileName();
93+ });
94+ EXPECT_EQ(result[0], audio2);
95+ EXPECT_EQ(result[1], audio1);
96 }
97
98 TEST_F(MediaStoreTest, utils) {

Subscribers

People subscribed via source and target branches