mediascanner

Merge lp:~jpakkane/mediascanner/immutable-mediafile into lp:mediascanner/v2

  1. immutable-mediafile
  2. Merge into v2
Proposed by Jussi Pakkanen
Status: Merged
Approved by: James Henstridge
Approved revision: 198
Merged at revision: 189
Proposed branch: lp:~jpakkane/mediascanner/immutable-mediafile
Merge into: lp:mediascanner/v2
Diff against target: 680 lines (+382/-91)
12 files modified
src/daemon/MetadataExtractor.cc (+21/-17)
src/mediascanner/CMakeLists.txt (+1/-0)
src/mediascanner/MediaFile.cc (+3/-53)
src/mediascanner/MediaFile.hh (+4/-12)
src/mediascanner/MediaFileBuilder.cc (+116/-0)
src/mediascanner/MediaFileBuilder.hh (+83/-0)
src/mediascanner/utils.cc (+18/-0)
src/mediascanner/utils.hh (+1/-0)
test/CMakeLists.txt (+4/-0)
test/basic.cc (+7/-4)
test/test_mediastore.cc (+4/-5)
test/test_mfbuilder.cc (+120/-0)
To merge this branch: bzr merge lp:~jpakkane/mediascanner/immutable-mediafile
Reviewer Review Type Date Requested Status
James Henstridge Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+197074@code.launchpad.net

Commit message

Made MediaFile immutable.

Description of the change

Made MediaFile immutable. This is done by creating a new MediaFileBuilder class that you can use to incrementally store MediaFile properties. MediaFile can then be built with one atomic step. This ensures no half-initialised MediaFiles can escape into the wild.

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

PASSED: Continuous integration, rev:190
http://jenkins.qa.ubuntu.com/job/mediascanner-v2-ci/5/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-amd64-ci/5
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/5
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/5/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-i386-ci/5

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner-v2-ci/5/rebuild

review: Approve (continuous-integration)
lp:~jpakkane/mediascanner/immutable-mediafile updated
191. By Jussi Pakkanen

Merged trunk.

192. By Jussi Pakkanen

Update tests.

193. By Jussi Pakkanen

Capitalisation fix.

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

PASSED: Continuous integration, rev:191
http://jenkins.qa.ubuntu.com/job/mediascanner-v2-ci/6/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-amd64-ci/6
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/6
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/6/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-i386-ci/6

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner-v2-ci/6/rebuild

review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:193
http://jenkins.qa.ubuntu.com/job/mediascanner-v2-ci/7/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-amd64-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/7
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/7/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-i386-ci/7

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner-v2-ci/7/rebuild

review: Approve (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

Is there any reason why you removed MediaFile.addUri()? I added it because I was using it in the scope.

review: Needs Information
Revision history for this message
James Henstridge (jamesh) wrote :

Also, as far as API designs go, what do you think of the one described here?

http://www.parashift.com/c++-faq/named-parameter-idiom.html

lp:~jpakkane/mediascanner/immutable-mediafile updated
194. By Jussi Pakkanen

Removed unnecessary includes.

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

PASSED: Continuous integration, rev:194
http://jenkins.qa.ubuntu.com/job/mediascanner-v2-ci/8/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/8
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/8/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-i386-ci/8

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner-v2-ci/8/rebuild

review: Approve (continuous-integration)
Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

MediaFileBuilder is already almost exactly that. It would just need to change the setXXX functions to return reference to *this. I can make that change if it is deemed useful.

The reason I moved the Uri function (I assume you meant getUri instead of addUri) is that after changing to mediafilebuilder the uri function was used only in one place (gstreamer pipeline building) and at that point a MediaFile had not been created yet.

If it is necessary I guess I can put it back, but since Mediascanner only deals with local files, it seems cleaner to me to only deal with filenames.

Revision history for this message
James Henstridge (jamesh) wrote :

We needed URIs in the daemon internals, and we needed URIs in some users of the media scanner (the scope is an obvious example, Drag and Drop also deals with URIs, etc).

It's not clear that having the method makes the interface more difficult to understand, so I'd prefer to keep it unless there is a good reason for it to go.

lp:~jpakkane/mediascanner/immutable-mediafile updated
195. By Jussi Pakkanen

Return of getUri.

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

PASSED: Continuous integration, rev:195
http://jenkins.qa.ubuntu.com/job/mediascanner-v2-ci/10/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-amd64-ci/10
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/10
        deb: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-armhf-ci/10/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mediascanner-v2-trusty-i386-ci/10

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mediascanner-v2-ci/10/rebuild

review: Approve (continuous-integration)
Revision history for this message
James Henstridge (jamesh) wrote :

[1]
Since the aim of this branch is to make MediaFile act like a value type, would it make sense to declare its data members to be const? That would ensure that the the MediaFile code itself doesn't start mutating the instance.

[2]
Since the big long argument list for the MediaFile constructor is a problem, would it be worth getting rid of it all together? At present, it is only being used in (a) MediaStore.make_media() and (b) tests, which could both be converted to MediaFileBuilder. This would also mean those pieces of code wouldn't immediately need updating whenever a new data member is added to MediaFile.

If we followed the example from the C++ FAQ, we could have a MediaFile constructor taking a Builder instance.

[3]
I'm on the fence on this one, but would it make sense for the MediaFileBuilder constructor to take the file name directly? This is the one field that is mandatory in a MediaFile, so it doesn't seem so bad to require it when creating the builder.

Revision history for this message
Jussi Pakkanen (jpakkane) wrote :

[1] This is something I thought about for a long time. It you make instance variables const, you can't copy them. Meaning you can't do this:

mf = some_func();

This would make Mediafile assigment atomic but not constant. Sometimes this is what you need. But I see the const issue as well. I guess we could start by making them const and unconsting them if necessary.

[2]

I'm a bit on the fence about this. I like being able to explicitly instantiate objects without other helper objects. This is a potential source of bugs, though.

[3]

Yes, change coming up.

lp:~jpakkane/mediascanner/immutable-mediafile updated
196. By Jussi Pakkanen

Make MediaFileBuilder take the filename in a constructor because that is mandatory.

197. By Jussi Pakkanen

Use initialisers in a constructor.

Revision history for this message
James Henstridge (jamesh) wrote :

Looks good. After discussing in person, I think we can clean up the other improvement issues post-merge.

review: Approve
lp:~jpakkane/mediascanner/immutable-mediafile updated
198. By Jussi Pakkanen

Fixed initialisation order.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/daemon/MetadataExtractor.cc'
--- src/daemon/MetadataExtractor.cc 2013-11-28 04:08:20 +0000
+++ src/daemon/MetadataExtractor.cc 2013-12-02 14:22:54 +0000
@@ -18,6 +18,8 @@
18 */18 */
1919
20#include "../mediascanner/MediaFile.hh"20#include "../mediascanner/MediaFile.hh"
21#include "../mediascanner/MediaFileBuilder.hh"
22#include "../mediascanner/utils.hh"
21#include "MetadataExtractor.hh"23#include "MetadataExtractor.hh"
2224
23#include <glib-object.h>25#include <glib-object.h>
@@ -32,6 +34,7 @@
3234
33using namespace std;35using namespace std;
3436
37
35struct MetadataExtractorPrivate {38struct MetadataExtractorPrivate {
36 std::unique_ptr<GstDiscoverer,void(*)(void *)> discoverer;39 std::unique_ptr<GstDiscoverer,void(*)(void *)> discoverer;
37 MetadataExtractorPrivate() : discoverer(nullptr, g_object_unref) {};40 MetadataExtractorPrivate() : discoverer(nullptr, g_object_unref) {};
@@ -102,7 +105,7 @@
102105
103static void106static void
104extract_tag_info (const GstTagList * list, const gchar * tag, gpointer user_data) {107extract_tag_info (const GstTagList * list, const gchar * tag, gpointer user_data) {
105 MediaFile *mf = (MediaFile *) user_data;108 MediaFileBuilder *mfb = (MediaFileBuilder *) user_data;
106 int i, num;109 int i, num;
107 string tagname(tag);110 string tagname(tag);
108111
@@ -113,23 +116,23 @@
113 val = gst_tag_list_get_value_index (list, tag, i);116 val = gst_tag_list_get_value_index (list, tag, i);
114 if (G_VALUE_HOLDS_STRING(val)) {117 if (G_VALUE_HOLDS_STRING(val)) {
115 if (tagname == GST_TAG_ARTIST)118 if (tagname == GST_TAG_ARTIST)
116 mf->setAuthor(g_value_get_string(val));119 mfb->setAuthor(g_value_get_string(val));
117 else if (tagname == GST_TAG_TITLE)120 else if (tagname == GST_TAG_TITLE)
118 mf->setTitle(g_value_get_string(val));121 mfb->setTitle(g_value_get_string(val));
119 else if (tagname == GST_TAG_ALBUM)122 else if (tagname == GST_TAG_ALBUM)
120 mf->setAlbum(g_value_get_string(val));123 mfb->setAlbum(g_value_get_string(val));
121 else if (tagname == GST_TAG_ALBUM_ARTIST)124 else if (tagname == GST_TAG_ALBUM_ARTIST)
122 mf->setAlbumArtist(g_value_get_string(val));125 mfb->setAlbumArtist(g_value_get_string(val));
123 } else if (G_VALUE_HOLDS(val, GST_TYPE_DATE_TIME)) {126 } else if (G_VALUE_HOLDS(val, GST_TYPE_DATE_TIME)) {
124 if (tagname == GST_TAG_DATE_TIME) {127 if (tagname == GST_TAG_DATE_TIME) {
125 GstDateTime *dt = static_cast<GstDateTime*>(g_value_get_boxed(val));128 GstDateTime *dt = static_cast<GstDateTime*>(g_value_get_boxed(val));
126 char *dt_string = gst_date_time_to_iso8601_string(dt);129 char *dt_string = gst_date_time_to_iso8601_string(dt);
127 mf->setDate(dt_string);130 mfb->setDate(dt_string);
128 g_free(dt_string);131 g_free(dt_string);
129 }132 }
130 } else if (G_VALUE_HOLDS_UINT(val)) {133 } else if (G_VALUE_HOLDS_UINT(val)) {
131 if (tagname == GST_TAG_TRACK_NUMBER) {134 if (tagname == GST_TAG_TRACK_NUMBER) {
132 mf->setTrackNumber(g_value_get_uint(val));135 mfb->setTrackNumber(g_value_get_uint(val));
133 }136 }
134 }137 }
135138
@@ -137,11 +140,12 @@
137}140}
138141
139MediaFile MetadataExtractor::extract(const DetectedFile &d) {142MediaFile MetadataExtractor::extract(const DetectedFile &d) {
140 MediaFile mf(d.filename);143 MediaFileBuilder mfb(d.filename);
141 mf.setETag(d.etag);144 mfb.setETag(d.etag);
142 mf.setContentType(d.content_type);145 mfb.setContentType(d.content_type);
143 mf.setType(d.type);146 mfb.setType(d.type);
144 string uri = mf.getUri();147 string uri = getUri(d.filename);
148
145 GError *error = nullptr;149 GError *error = nullptr;
146 unique_ptr<GstDiscovererInfo, void(*)(void *)> info(150 unique_ptr<GstDiscovererInfo, void(*)(void *)> info(
147 gst_discoverer_discover_uri(p->discoverer.get(), uri.c_str(), &error),151 gst_discoverer_discover_uri(p->discoverer.get(), uri.c_str(), &error),
@@ -151,21 +155,21 @@
151 g_error_free(error);155 g_error_free(error);
152156
153 string msg = "Discovery of file ";157 string msg = "Discovery of file ";
154 msg += mf.getFileName();158 msg += d.filename;
155 msg += " failed: ";159 msg += " failed: ";
156 msg += errortxt;160 msg += errortxt;
157 throw runtime_error(msg);161 throw runtime_error(msg);
158 }162 }
159163
160 if (gst_discoverer_info_get_result(info.get()) != GST_DISCOVERER_OK) {164 if (gst_discoverer_info_get_result(info.get()) != GST_DISCOVERER_OK) {
161 throw runtime_error("Unable to discover file " + mf.getFileName());165 throw runtime_error("Unable to discover file " + d.filename);
162 }166 }
163167
164 const GstTagList *tags = gst_discoverer_info_get_tags(info.get());168 const GstTagList *tags = gst_discoverer_info_get_tags(info.get());
165 if (tags != NULL) {169 if (tags != NULL) {
166 gst_tag_list_foreach(tags, extract_tag_info, &mf);170 gst_tag_list_foreach(tags, extract_tag_info, &mfb);
167 }171 }
168 mf.setDuration(static_cast<int>(172 mfb.setDuration(static_cast<int>(
169 gst_discoverer_info_get_duration(info.get())/GST_SECOND));173 gst_discoverer_info_get_duration(info.get())/GST_SECOND));
170 return mf;174 return mfb.build();
171}175}
172176
=== modified file 'src/mediascanner/CMakeLists.txt'
--- src/mediascanner/CMakeLists.txt 2013-11-14 15:04:14 +0000
+++ src/mediascanner/CMakeLists.txt 2013-12-02 14:22:54 +0000
@@ -1,5 +1,6 @@
1add_library(mediascanner SHARED1add_library(mediascanner SHARED
2 MediaFile.cc2 MediaFile.cc
3 MediaFileBuilder.cc
3 Album.cc4 Album.cc
4 MediaStore.cc5 MediaStore.cc
5 utils.cc6 utils.cc
67
=== modified file 'src/mediascanner/MediaFile.cc'
--- src/mediascanner/MediaFile.cc 2013-11-25 07:53:45 +0000
+++ src/mediascanner/MediaFile.cc 2013-12-02 14:22:54 +0000
@@ -17,9 +17,8 @@
17 * along with this program. If not, see <http://www.gnu.org/licenses/>.17 * along with this program. If not, see <http://www.gnu.org/licenses/>.
18 */18 */
1919
20#include <stdexcept>
21#include <glib.h>
22#include "MediaFile.hh"20#include "MediaFile.hh"
21#include "utils.hh"
2322
24using namespace std;23using namespace std;
2524
@@ -73,44 +72,8 @@
73 return type;72 return type;
74}73}
7574
76void MediaFile::setContentType(const std::string &content_type) noexcept {75std::string MediaFile::getUri() const {
77 this->content_type = content_type;76 return ::getUri(filename);
78}
79
80void MediaFile::setETag(const std::string &etag) noexcept {
81 this->etag = etag;
82}
83
84void MediaFile::setTitle(const std::string &title) noexcept {
85 this->title = title;
86}
87
88void MediaFile::setAuthor(const std::string &author) noexcept {
89 this->author = author;
90}
91
92void MediaFile::setAlbum(const std::string &album) noexcept {
93 this->album = album;
94}
95
96void MediaFile::setAlbumArtist(const std::string &album_artist) noexcept {
97 this->album_artist = album_artist;
98}
99
100void MediaFile::setDate(const std::string &date) noexcept {
101 this->date = date;
102}
103
104void MediaFile::setTrackNumber(int track_number) noexcept {
105 this->track_number = track_number;
106}
107
108void MediaFile::setDuration(int duration) noexcept {
109 this->duration = duration;
110}
111
112void MediaFile::setType(MediaType type) noexcept {
113 this->type = type;
114}77}
11578
116bool MediaFile::operator==(const MediaFile &other) const {79bool MediaFile::operator==(const MediaFile &other) const {
@@ -132,16 +95,3 @@
132 return !(*this == other);95 return !(*this == other);
133}96}
13497
135std::string MediaFile::getUri() const {
136 GError *error = NULL;
137 char *uristr = g_filename_to_uri(filename.c_str(), "", &error);
138 if (error) {
139 string msg("Could not build URI: ");
140 msg += error->message;
141 g_error_free(error);
142 throw runtime_error(msg);
143 }
144 string uri(uristr);
145 g_free(uristr);
146 return uri;
147}
14898
=== modified file 'src/mediascanner/MediaFile.hh'
--- src/mediascanner/MediaFile.hh 2013-11-25 07:53:45 +0000
+++ src/mediascanner/MediaFile.hh 2013-12-02 14:22:54 +0000
@@ -40,24 +40,16 @@
40 const std::string& getAlbum() const noexcept;40 const std::string& getAlbum() const noexcept;
41 const std::string& getAlbumArtist() const noexcept;41 const std::string& getAlbumArtist() const noexcept;
42 const std::string& getDate() const noexcept;42 const std::string& getDate() const noexcept;
43 std::string getUri() const;
44
43 int getTrackNumber() const noexcept;45 int getTrackNumber() const noexcept;
44 int getDuration() const noexcept;46 int getDuration() const noexcept;
45 MediaType getType() const noexcept;47 MediaType getType() const noexcept;
46 bool operator==(const MediaFile &other) const;48 bool operator==(const MediaFile &other) const;
47 bool operator!=(const MediaFile &other) const;49 bool operator!=(const MediaFile &other) const;
4850
49 void setContentType(const std::string& content_type) noexcept;51 // There are no setters. MediaFiles are immutable.
50 void setETag(const std::string& etag) noexcept;52 // For piecewise construction use MediaFileBuilder.
51 void setTitle(const std::string& title) noexcept;
52 void setAuthor(const std::string& author) noexcept;
53 void setAlbum(const std::string& album) noexcept;
54 void setAlbumArtist(const std::string& album_artist) noexcept;
55 void setDate(const std::string& date) noexcept;
56 void setTrackNumber(int track_number) noexcept;
57 void setDuration(int duration) noexcept;
58 void setType(MediaType type) noexcept;
59
60 std::string getUri() const;
6153
62private:54private:
63 std::string filename;55 std::string filename;
6456
=== added file 'src/mediascanner/MediaFileBuilder.cc'
--- src/mediascanner/MediaFileBuilder.cc 1970-01-01 00:00:00 +0000
+++ src/mediascanner/MediaFileBuilder.cc 2013-12-02 14:22:54 +0000
@@ -0,0 +1,116 @@
1/*
2 * Copyright (C) 2013 Canonical, Ltd.
3 *
4 * Authors:
5 * Jussi Pakkanen <jussi.pakkanen@canonical.com>
6 *
7 * This library is free software; you can redistribute it and/or modify it under
8 * the terms of version 3 of the GNU General Public License as published
9 * by the Free Software Foundation.
10 *
11 * This library is distributed in the hope that it will be useful, but WITHOUT
12 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
13 * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
14 * details.
15 *
16 * You should have received a copy of the GNU General Public License
17 * along with this program. If not, see <http://www.gnu.org/licenses/>.
18 */
19
20#include"MediaFileBuilder.hh"
21#include"MediaFile.hh"
22#include<stdexcept>
23
24MediaFileBuilder::MediaFileBuilder(const std::string &fname) {
25 filename = fname;
26}
27
28MediaFileBuilder::MediaFileBuilder(const MediaFile &mf) :
29 type(mf.getType()),
30 filename(mf.getFileName()),
31 content_type(mf.getContentType()),
32 etag(mf.getETag()),
33 title(mf.getTitle()),
34 date(mf.getDate()),
35 author(mf.getAuthor()),
36 album(mf.getAlbum()),
37 album_artist(mf.getAlbumArtist()),
38 track_number(mf.getTrackNumber()),
39 duration(mf.getDuration()) {
40}
41
42MediaFile MediaFileBuilder::build() const {
43 return MediaFile(filename, content_type, etag, title, date, author,
44 album, album_artist, track_number, duration, type);
45}
46
47void MediaFileBuilder::setType(MediaType t) {
48 if(type_set)
49 throw std::invalid_argument("Tried to set type when it was already set.");
50 type = t;
51 type_set = true;
52}
53
54void MediaFileBuilder::setETag(const std::string &e) {
55 if(etag_set)
56 throw std::invalid_argument("Tried to set filename when it was already set.");
57 etag = e;
58 etag_set = true;
59
60}
61void MediaFileBuilder::setContentType(const std::string &c) {
62 if(content_type_set)
63 throw std::invalid_argument("Tried to set filename when it was already set.");
64 content_type = c;
65 content_type_set = true;
66
67}
68
69void MediaFileBuilder::setTitle(const std::string &t) {
70 if(title_set)
71 throw std::invalid_argument("Tried to set title when it was already set.");
72 title = t;
73 title_set = true;
74}
75
76void MediaFileBuilder::setDate(const std::string &d) {
77 if(date_set)
78 throw std::invalid_argument("Tried to set date when it was already set.");
79 date = d;
80 date_set = true;
81}
82
83void MediaFileBuilder::setAuthor(const std::string &a) {
84 if(author_set)
85 throw std::invalid_argument("Tried to set author when it was already set.");
86 author = a;
87 author_set = true;
88}
89
90void MediaFileBuilder::setAlbum(const std::string &a) {
91 if(album_set)
92 throw std::invalid_argument("Tried to set album when it was already set.");
93 album = a;
94 album_set = true;
95}
96
97void MediaFileBuilder::setAlbumArtist(const std::string &a) {
98 if(album_artist_set)
99 throw std::invalid_argument("Tried to set album artist when it was already set.");
100 album_artist = a;
101 album_artist_set = true;
102}
103
104void MediaFileBuilder::setTrackNumber(int n) {
105 if(track_number_set)
106 throw std::invalid_argument("Tried to set track number when it was already set.");
107 track_number = n;
108 track_number_set = true;
109}
110
111void MediaFileBuilder::setDuration(int n) {
112 if(duration_set)
113 throw std::invalid_argument("Tried to set duration when it was already set.");
114 duration = n;
115 duration_set = true;
116}
0117
=== added file 'src/mediascanner/MediaFileBuilder.hh'
--- src/mediascanner/MediaFileBuilder.hh 1970-01-01 00:00:00 +0000
+++ src/mediascanner/MediaFileBuilder.hh 2013-12-02 14:22:54 +0000
@@ -0,0 +1,83 @@
1/*
2 * Copyright (C) 2013 Canonical, Ltd.
3 *
4 * Authors:
5 * Jussi Pakkanen <jussi.pakkanen@canonical.com>
6 *
7 * This library is free software; you can redistribute it and/or modify it under
8 * the terms of version 3 of the GNU General Public License as published
9 * by the Free Software Foundation.
10 *
11 * This library is distributed in the hope that it will be useful, but WITHOUT
12 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
13 * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
14 * details.
15 *
16 * You should have received a copy of the GNU General Public License
17 * along with this program. If not, see <http://www.gnu.org/licenses/>.
18 */
19
20#ifndef MEDIAFILEBUILDER_H_
21#define MEDIAFILEBUILDER_H_
22
23#include"scannercore.hh"
24#include<string>
25
26class MediaFile;
27
28class MediaFileBuilder final {
29public:
30 MediaFileBuilder(const std::string &filename);
31 MediaFileBuilder(const MediaFile &mf);
32 MediaFileBuilder(const MediaFileBuilder &) = delete;
33 MediaFileBuilder& operator=(MediaFileBuilder &) = delete;
34
35 MediaFile build() const;
36
37 void setType(MediaType t);
38 void setETag(const std::string &e);
39 void setContentType(const std::string &c);
40 void setTitle(const std::string &t);
41 void setDate(const std::string &d);
42 void setAuthor(const std::string &a);
43 void setAlbum(const std::string &a);
44 void setAlbumArtist(const std::string &a);
45 void setTrackNumber(int n);
46 void setDuration(int d);
47
48private:
49 bool type_set = false;
50 MediaType type = UnknownMedia;
51
52 std::string filename;
53
54 bool content_type_set = false;
55 std::string content_type;
56
57 bool etag_set = false;
58 std::string etag;
59
60 bool title_set = false;
61 std::string title;
62
63 bool date_set = false;
64 std::string date;
65
66 bool author_set = false;
67 std::string author;
68
69 bool album_set = false;
70 std::string album;
71
72 bool album_artist_set = false;
73 std::string album_artist;
74
75 int track_number = 0;
76 bool track_number_set = false;
77
78 int duration = 0;
79 bool duration_set = false;
80};
81
82
83#endif
084
=== modified file 'src/mediascanner/utils.cc'
--- src/mediascanner/utils.cc 2013-11-12 04:35:16 +0000
+++ src/mediascanner/utils.cc 2013-12-02 14:22:54 +0000
@@ -20,6 +20,8 @@
20#include"utils.hh"20#include"utils.hh"
2121
22#include<vector>22#include<vector>
23#include<stdexcept>
24#include<glib.h>
2325
24std::string sqlQuote(const std::string &input) {26std::string sqlQuote(const std::string &input) {
25 std::vector<char> out;27 std::vector<char> out;
@@ -67,3 +69,19 @@
67 }69 }
68 return result;70 return result;
69}71}
72
73std::string getUri(const std::string &filename) {
74 GError *error = NULL;
75 char *uristr = g_filename_to_uri(filename.c_str(), "", &error);
76 if (error) {
77 std::string msg("Could not build URI: ");
78 msg += error->message;
79 g_error_free(error);
80 throw std::runtime_error(msg);
81 }
82
83 std::string uri(uristr);
84 g_free(uristr);
85 return uri;
86}
87
7088
=== modified file 'src/mediascanner/utils.hh'
--- src/mediascanner/utils.hh 2013-11-12 04:35:16 +0000
+++ src/mediascanner/utils.hh 2013-12-02 14:22:54 +0000
@@ -24,5 +24,6 @@
2424
25std::string sqlQuote(const std::string &input);25std::string sqlQuote(const std::string &input);
26std::string filenameToTitle(const std::string &filename);26std::string filenameToTitle(const std::string &filename);
27std::string getUri(const std::string &filename);
2728
28#endif29#endif
2930
=== modified file 'test/CMakeLists.txt'
--- test/CMakeLists.txt 2013-11-18 10:23:49 +0000
+++ test/CMakeLists.txt 2013-12-02 14:22:54 +0000
@@ -38,3 +38,7 @@
38add_executable(test_sqliteutils test_sqliteutils.cc)38add_executable(test_sqliteutils test_sqliteutils.cc)
39target_link_libraries(test_sqliteutils gtest ${MEDIASCANNER_LIBRARIES})39target_link_libraries(test_sqliteutils gtest ${MEDIASCANNER_LIBRARIES})
40add_test(test_sqliteutils test_sqliteutils)40add_test(test_sqliteutils test_sqliteutils)
41
42add_executable(test_mfbuilder test_mfbuilder.cc)
43target_link_libraries(test_mfbuilder gtest mediascanner)
44add_test(test_mfbuilder test_mfbuilder)
4145
=== modified file 'test/basic.cc'
--- test/basic.cc 2013-11-28 04:08:20 +0000
+++ test/basic.cc 2013-12-02 14:22:54 +0000
@@ -18,6 +18,7 @@
18 */18 */
1919
20#include <mediascanner/MediaFile.hh>20#include <mediascanner/MediaFile.hh>
21#include <mediascanner/MediaFileBuilder.hh>
21#include <mediascanner/MediaStore.hh>22#include <mediascanner/MediaStore.hh>
22#include <daemon/MetadataExtractor.hh>23#include <daemon/MetadataExtractor.hh>
23#include <daemon/SubtreeWatcher.hh>24#include <daemon/SubtreeWatcher.hh>
@@ -181,8 +182,9 @@
181 MediaFile media = store.lookup(outfile);182 MediaFile media = store.lookup(outfile);
182 EXPECT_NE(media.getETag(), "");183 EXPECT_NE(media.getETag(), "");
183 EXPECT_EQ(media.getTitle(), "track1");184 EXPECT_EQ(media.getTitle(), "track1");
184 media.setTitle("something else");185 MediaFileBuilder mfb(media);
185 store.insert(media);186 mfb.setTitle("something else");
187 store.insert(mfb.build());
186188
187 /* Scan again, and note that the data hasn't been updated */189 /* Scan again, and note that the data hasn't been updated */
188 scanFiles(store, testdir, AudioMedia);190 scanFiles(store, testdir, AudioMedia);
@@ -190,8 +192,9 @@
190 EXPECT_EQ(media.getTitle(), "something else");192 EXPECT_EQ(media.getTitle(), "something else");
191193
192 /* Now change the stored etag, to trigger an update */194 /* Now change the stored etag, to trigger an update */
193 media.setETag("old-etag");195 MediaFileBuilder mfb2(media);
194 store.insert(media);196 mfb2.setETag("old-etag");
197 store.insert(mfb2.build());
195 scanFiles(store, testdir, AudioMedia);198 scanFiles(store, testdir, AudioMedia);
196 media = store.lookup(outfile);199 media = store.lookup(outfile);
197 EXPECT_EQ(media.getTitle(), "track1");200 EXPECT_EQ(media.getTitle(), "track1");
198201
=== modified file 'test/test_mediastore.cc'
--- test/test_mediastore.cc 2013-11-28 03:54:39 +0000
+++ test/test_mediastore.cc 2013-12-02 14:22:54 +0000
@@ -47,6 +47,10 @@
47TEST_F(MediaStoreTest, init) {47TEST_F(MediaStoreTest, init) {
48 MediaStore store(":memory:", MS_READ_WRITE);48 MediaStore store(":memory:", MS_READ_WRITE);
49}49}
50TEST_F(MediaStoreTest, mediafile_uri) {
51 MediaFile media("/path/to/file.ogg");
52 EXPECT_EQ(media.getUri(), "file:///path/to/file.ogg");
53}
5054
51TEST_F(MediaStoreTest, equality) {55TEST_F(MediaStoreTest, equality) {
52 MediaFile audio1("a", "type", "etag", "1900", "b", "c", "d", "e", 1, 5, AudioMedia);56 MediaFile audio1("a", "type", "etag", "1900", "b", "c", "d", "e", 1, 5, AudioMedia);
@@ -64,11 +68,6 @@
64 EXPECT_NE(audio2, video2);68 EXPECT_NE(audio2, video2);
65}69}
6670
67TEST_F(MediaStoreTest, mediafile_uri) {
68 MediaFile media("/path/to/file.ogg");
69 EXPECT_EQ(media.getUri(), "file:///path/to/file.ogg");
70}
71
72TEST_F(MediaStoreTest, lookup) {71TEST_F(MediaStoreTest, lookup) {
73 MediaFile audio("aaa", "type", "etag", "bbb bbb", "1900-01-01", "ccc", "ddd", "eee", 3, 5, AudioMedia);72 MediaFile audio("aaa", "type", "etag", "bbb bbb", "1900-01-01", "ccc", "ddd", "eee", 3, 5, AudioMedia);
74 MediaStore store(":memory:", MS_READ_WRITE);73 MediaStore store(":memory:", MS_READ_WRITE);
7574
=== added file 'test/test_mfbuilder.cc'
--- test/test_mfbuilder.cc 1970-01-01 00:00:00 +0000
+++ test/test_mfbuilder.cc 2013-12-02 14:22:54 +0000
@@ -0,0 +1,120 @@
1/*
2 * Copyright (C) 2013 Canonical, Ltd.
3 *
4 * Authors:
5 * Jussi Pakkanen <jussi.pakkanen@canonical.com>
6 *
7 * This library is free software; you can redistribute it and/or modify it under
8 * the terms of version 3 of the GNU General Public License as published
9 * by the Free Software Foundation.
10 *
11 * This library is distributed in the hope that it will be useful, but WITHOUT
12 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
13 * FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
14 * details.
15 *
16 * You should have received a copy of the GNU General Public License
17 * along with this program. If not, see <http://www.gnu.org/licenses/>.
18 */
19
20#include<gtest/gtest.h>
21#include<stdexcept>
22#include"mediascanner/MediaFile.hh"
23#include"mediascanner/MediaFileBuilder.hh"
24
25/**
26 * This is a helper class to build MediaFiles. Since we want them
27 * to be immutable and always valid, the user needs to always list
28 * all variables in the constructor. This is cumbersome so this class
29 * allows you to gather them one by one and then finally construct
30 * a fully valid MediaFile.
31 *
32 * If you try to assign the same property twice, an exception is thrown.
33 * This means that MediaFileBuilders are meant to build only one
34 * MediaFile. To build a new one create a new MediaFileBuilder. This is to
35 * ensure that no state leaks from the first MediaFile to the second.
36 */
37
38class MFBTest : public ::testing::Test {
39 protected:
40 MFBTest() {
41 }
42
43 virtual ~MFBTest() {
44 }
45
46 virtual void SetUp() {
47 }
48
49 virtual void TearDown() {
50 }
51};
52
53TEST_F(MFBTest, basic) {
54 MediaType type(AudioMedia);
55 std::string fname("abc");
56 std::string title("def");
57 std::string date("ghi");
58 std::string author("jkl");
59 std::string album("mno");
60 std::string album_artist("pqr");
61 std::string etag("stu");
62 std::string content_type("vwx");
63 int track_number = 13;
64 int duration = 99;
65
66 MediaFileBuilder b(fname);
67
68 b.setType(type);
69 ASSERT_THROW(b.setType(type), std::invalid_argument);
70
71 b.setTitle(title);
72 ASSERT_THROW(b.setTitle(fname), std::invalid_argument);
73
74 b.setDate(date);
75 ASSERT_THROW(b.setDate(date), std::invalid_argument);
76
77 b.setAuthor(author);
78 ASSERT_THROW(b.setAuthor(author), std::invalid_argument);
79
80 b.setAlbum(album);
81 ASSERT_THROW(b.setAlbum(album), std::invalid_argument);
82
83 b.setAlbumArtist(album_artist);
84 ASSERT_THROW(b.setAlbumArtist(album_artist), std::invalid_argument);
85
86 b.setTrackNumber(track_number);
87 ASSERT_THROW(b.setTrackNumber(track_number), std::invalid_argument);
88
89 b.setDuration(duration);
90 ASSERT_THROW(b.setDuration(duration), std::invalid_argument);
91
92 b.setETag(etag);
93 ASSERT_THROW(b.setETag(etag), std::invalid_argument);
94
95 b.setContentType(content_type);
96 ASSERT_THROW(b.setContentType(content_type), std::invalid_argument);
97
98 // Now see if data survives a round trip.
99 MediaFile mf = b.build();
100 ASSERT_EQ(mf.getType(), type);
101 ASSERT_EQ(mf.getFileName(), fname);
102 ASSERT_EQ(mf.getTitle(), title);
103 ASSERT_EQ(mf.getDate(), date);
104 ASSERT_EQ(mf.getAuthor(), author);
105 ASSERT_EQ(mf.getAlbum(), album);
106 ASSERT_EQ(mf.getAlbumArtist(), album_artist);
107 ASSERT_EQ(mf.getTrackNumber(), track_number);
108 ASSERT_EQ(mf.getDuration(), duration);
109 ASSERT_EQ(mf.getETag(), etag);
110 ASSERT_EQ(mf.getContentType(), content_type);
111
112 MediaFileBuilder mfb2(mf);
113 MediaFile mf2 = mfb2.build();
114 ASSERT_EQ(mf, mf2);
115}
116
117int main(int argc, char **argv) {
118 ::testing::InitGoogleTest(&argc, argv);
119 return RUN_ALL_TESTS();
120}

Subscribers

People subscribed via source and target branches