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

[James Henstridge]

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

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.

[Michi Henning (michihenning)]

Looks good!

review: Approve

=== modified file 'src/mediascanner/MediaStore.cc'
--- src/mediascanner/MediaStore.cc	2015-07-14 01:34:56 +0000
+++ src/mediascanner/MediaStore.cc	2015-10-07 07:13:13 +0000
@@ -46,7 +46,7 @@
 
 // Increment this whenever changing db schema.
 // It will cause dbstore to rebuild its tables.
-static const int schemaVersion = 9;
+static const int schemaVersion = 10;
 
 struct MediaStorePrivate {
     sqlite3 *db;
@@ -322,7 +322,6 @@
 CREATE INDEX media_mtime_idx ON media(type, mtime);
 
 CREATE TABLE media_attic (
-    id INTEGER PRIMARY KEY,
     filename TEXT UNIQUE NOT NULL,
     content_type TEXT,
     etag TEXT,
@@ -918,7 +917,9 @@
 
 void MediaStorePrivate::archiveItems(const std::string &prefix) {
     const char *templ = R"(BEGIN TRANSACTION;
-INSERT INTO media_attic SELECT * FROM media WHERE filename LIKE %s;
+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)
+  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
+    FROM media WHERE filename LIKE %s;
 DELETE FROM media WHERE filename LIKE %s;
 COMMIT;
 )";
@@ -935,7 +936,9 @@
 
 void MediaStorePrivate::restoreItems(const std::string &prefix) {
     const char *templ = R"(BEGIN TRANSACTION;
-INSERT INTO media SELECT * FROM media_attic WHERE filename LIKE %s;
+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)
+  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
+    FROM media_attic WHERE filename LIKE %s;
 DELETE FROM media_attic WHERE filename LIKE %s;
 COMMIT;
 )";

=== modified file 'test/test_mediastore.cc'
--- test/test_mediastore.cc	2015-07-14 01:34:56 +0000
+++ test/test_mediastore.cc	2015-10-07 07:13:13 +0000
@@ -24,9 +24,10 @@
 #include <mediascanner/MediaStore.hh>
 #include <mediascanner/internal/utils.hh>
 
-#include<stdexcept>
-#include<cstdio>
-#include<string>
+#include <algorithm>
+#include <stdexcept>
+#include <cstdio>
+#include <string>
 #include <gtest/gtest.h>
 
 using namespace std;
@@ -488,7 +489,23 @@
 TEST_F(MediaStoreTest, unmount) {
     MediaFile audio1 = MediaFileBuilder("/media/username/dir/fname.ogg")
         .setType(AudioMedia)
-        .setTitle("bbb bbb");
+        .setETag("etag")
+        .setContentType("audio/ogg")
+        .setTitle("bbb bbb")
+        .setDate("2015-01-01")
+        .setAuthor("artist")
+        .setAlbum("album")
+        .setAlbumArtist("album_artist")
+        .setGenre("genre")
+        .setDiscNumber(5)
+        .setTrackNumber(10)
+        .setDuration(42)
+        .setWidth(640)
+        .setHeight(480)
+        .setLatitude(8.0)
+        .setLongitude(4.0)
+        .setHasThumbnail(true)
+        .setModificationTime(10000);
     MediaFile audio2 = MediaFileBuilder("/home/username/Music/fname.ogg")
         .setType(AudioMedia)
         .setTitle("bbb bbb");
@@ -507,6 +524,12 @@
     store.restoreItems("/media/username");
     result = store.query("bbb", AudioMedia, filter);
     ASSERT_EQ(result.size(), 2);
+    std::sort(result.begin(), result.end(),
+              [](const MediaFile &m1, const MediaFile &m2) -> bool {
+                  return m1.getFileName() < m2.getFileName();
+              });
+    EXPECT_EQ(result[0], audio2);
+    EXPECT_EQ(result[1], audio1);
 }
 
 TEST_F(MediaStoreTest, utils) {