[libmediaart/media-art-types] cache: Support more media art types than just 'album'

[Sam Thursfield <sthursfield src gnome org>]

commit 4d085934279728f6ac0d0cdda50def4a06515d77
Author: Sam Thursfield <sam thursfield codethink co uk>
Date:   Fri Oct 11 16:02:26 2013 +0100

    cache: Support more media art types than just 'album'
    
    The media art storage spec
    https://wiki.gnome.org/MediaArtStorageSpec#Identifiers specifies the
    following possible values for prefix: 'album', 'artist', 'podcast',
    'radio', 'track'.
    
    This commit adds support for all of these except 'track', which will
    currently raise a g_warning() if used to advise the caller that it is
    not supported.
    
    Rather than only supporting a fixed list of identifiers, any unknown
    prefix is accepted and handled in the same way as 'artist', 'radio' and
    'podcast' media art.
    
    This commit also fixes
    https://bugzilla.gnome.org/show_bug.cgi?id=705834 where the path for
    'album' media art where artist == NULL was calculated incorrectly.

 libmediaart/cache.c  |   39 +++++++++++++++++------
 tests/mediaarttest.c |   84 +++++++++++++++++++++++++++++++++++---------------
 2 files changed, 88 insertions(+), 35 deletions(-)
---
diff --git a/libmediaart/cache.c b/libmediaart/cache.c
index e2592b6..ef3426b 100644
--- a/libmediaart/cache.c
+++ b/libmediaart/cache.c
@@ -228,7 +228,6 @@ media_art_get_file (const gchar  *artist,
                    GFile       **local_file)
 {
        const gchar *space_checksum = "7215ee9c7d9dc229d2921a40e899ec5f";
-       const gchar *a, *b;
 
        gchar *art_filename;
        gchar *dir, *filename;
@@ -236,6 +235,7 @@ media_art_get_file (const gchar  *artist,
        gchar *artist_stripped, *title_stripped;
        gchar *artist_norm, *title_norm;
        gchar *artist_checksum = NULL, *title_checksum = NULL;
+       gboolean title_only;
 
        /* http://live.gnome.org/MediaArtStorageSpec */
 
@@ -247,7 +247,21 @@ media_art_get_file (const gchar  *artist,
                *local_file = NULL;
        }
 
-       if (!artist && !title) {
+       g_return_if_fail (prefix != NULL);
+
+       if (strcmp(prefix, "track") == 0) {
+               /* Specified by media art storage spec, but not implemented yet */
+               g_warning ("libmediaart does not support 'track' type media art.");
+               return;
+       }
+
+       if (strcmp (prefix, "album") == 0) {
+               title_only = FALSE;
+       } else {
+               title_only = TRUE;
+       }
+
+       if ((title_only && !title) || (!title && !artist)) {
                return;
        }
 
@@ -277,16 +291,21 @@ media_art_get_file (const gchar  *artist,
                g_mkdir_with_parents (dir, 0770);
        }
 
-       if (artist) {
-               a = artist_checksum;
-               b = title ? title_checksum : space_checksum;
+       if (title_only) {
+               /* The media art spec describes two types other than "artist":
+                * "podcast", and "radio", both of which have just the "title"
+                * attribute. For the sake of flexibility we allow any prefix to be
+                * used here, so applications can implement simple extentions like a
+                * 'video' media art type.
+                */
+               art_filename = g_strdup_printf ("%s-%s-%s.jpeg", prefix,
+                                               title_checksum, space_checksum);
        } else {
-               a = title_checksum;
-               b = space_checksum;
+               art_filename = g_strdup_printf ("%s-%s-%s.jpeg", prefix,
+                                               artist ? artist_checksum : space_checksum,
+                                               title ? title_checksum : space_checksum);
        }
 
-       art_filename = g_strdup_printf ("%s-%s-%s.jpeg", prefix ? prefix : "album", a, b);
-
        if (artist) {
                g_free (artist_checksum);
                g_free (artist_stripped);
@@ -392,7 +411,7 @@ media_art_remove (const gchar *artist,
        gchar *target = NULL;
        gchar *album_path = NULL;
 
-       g_return_if_fail (artist != NULL && artist[0] != '\0');
+       g_return_val_if_fail (artist != NULL && artist[0] != '\0', FALSE);
 
        dirname = g_build_filename (g_get_user_cache_dir (), "media-art", NULL);
 
diff --git a/tests/mediaarttest.c b/tests/mediaarttest.c
index e236713..f95afc7 100644
--- a/tests/mediaarttest.c
+++ b/tests/mediaarttest.c
@@ -71,27 +71,45 @@ test_mediaart_stripping_null (void)
         //g_assert (!mediaart_strip_invalid_entities (NULL));
 }
 
-struct {
+typedef struct {
+        const gchar *prefix;
         const gchar *artist;
-        const gchar *album;
+        const gchar *title;
         const gchar *filename;
-} mediaart_test_cases [] = {
-        {"Beatles", "Sgt. Pepper", 
-         "album-2a9ea35253dbec60e76166ec8420fbda-cfba4326a32b44b8760b3a2fc827a634.jpeg"},
+} MediaArtTestCase;
+
+MediaArtTestCase mediaart_test_cases [] = {
+        { "album", "Beatles", "Sgt. Pepper",
+          "album-2a9ea35253dbec60e76166ec8420fbda-cfba4326a32b44b8760b3a2fc827a634.jpeg"},
 
-        { "", "sgt. pepper",
+        { "album", "", "sgt. pepper",
           "album-d41d8cd98f00b204e9800998ecf8427e-cfba4326a32b44b8760b3a2fc827a634.jpeg"},
 
-        { " ", "sgt. pepper",
+        { "album", " ", "sgt. pepper",
           "album-d41d8cd98f00b204e9800998ecf8427e-cfba4326a32b44b8760b3a2fc827a634.jpeg"},
 
-        { NULL, "sgt. pepper",
-          "album-cfba4326a32b44b8760b3a2fc827a634-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"}, 
+        { "album", NULL, "sgt. pepper",
+          "album-7215ee9c7d9dc229d2921a40e899ec5f-cfba4326a32b44b8760b3a2fc827a634.jpeg"},
 
-        { "Beatles", NULL,
+        { "album", "Beatles", NULL,
           "album-2a9ea35253dbec60e76166ec8420fbda-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"},
 
-        { NULL, NULL, NULL }
+        { "album", NULL, NULL, NULL },
+
+        { "podcast", NULL, "Podcast example",
+          "podcast-10ca71a13bbd1a2af179f6d5a4dea118-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"},
+
+        { "radio", NULL, "Radio Free Europe",
+          "radio-79577732dda605d0f953f6479ff1f42e-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"},
+
+        { "radio", "Artist is ignored", NULL, NULL },
+
+        { "x-video", NULL, "Test extension of spec",
+          "x-video-51110ae14ce4bbeb68335366289acdd1-7215ee9c7d9dc229d2921a40e899ec5f.jpeg"},
+
+        /* Currently we raise a warning to advise the user that we don't
+         * support this part of the media art spec, which isn't tested here. */
+        /*{ "track", "Ignored", "Ignored", NULL},*/
 };
 
 static void
@@ -100,21 +118,37 @@ test_mediaart_location (void)
         gchar *path = NULL, *local_uri = NULL;
         gchar *expected;
         gint i;
-     
-        for (i = 0; mediaart_test_cases[i].filename != NULL; i++) {
-                media_art_get_path (mediaart_test_cases[i].artist,
-                                    mediaart_test_cases[i].album,
-                                    "album",
+
+        for (i = 0; i < G_N_ELEMENTS(mediaart_test_cases); i++) {
+                MediaArtTestCase *testcase = &mediaart_test_cases[i];
+
+                media_art_get_path (testcase->artist,
+                                    testcase->title,
+                                    testcase->prefix,
                                     "file:///home/test/a.mp3",
                                     &path,
                                     &local_uri);
-                expected = g_build_path (G_DIR_SEPARATOR_S, 
-                                         g_get_user_cache_dir (),
-                                         "media-art",
-                                         mediaart_test_cases[i].filename, 
-                                         NULL);
-                g_assert_cmpstr (path, ==, expected);
-                
+
+                if (testcase->filename == NULL) {
+                    expected = NULL;
+                } else {
+                    expected = g_build_path (G_DIR_SEPARATOR_S,
+                                             g_get_user_cache_dir (),
+                                             "media-art",
+                                             testcase->filename,
+                                             NULL);
+                }
+
+                if (g_strcmp0(path, expected) != 0) {
+                    g_error ("Incorrect path for prefix: '%s' artist: '%s' "
+                             "title: '%s'. Expected %s, got %s",
+                             testcase->prefix,
+                             testcase->artist,
+                             testcase->title,
+                             expected,
+                             path);
+                }
+
                 g_free (expected);
                 g_free (path);
                 g_free (local_uri);
@@ -142,8 +176,8 @@ test_mediaart_location_path (void)
 
         /* Use path instead of URI */
         media_art_get_path (mediaart_test_cases[0].artist,
-                            mediaart_test_cases[0].album,
-                            "album",
+                            mediaart_test_cases[0].title,
+                            mediaart_test_cases[0].prefix,
                             "/home/test/a.mp3",
                             &path,
                             &local_uri);