[libmediaart/api-cleanup: 4/11] extract: Refactor media_art_process_file() to simplify the function

[Martyn James Russell <mr src gnome org>]

commit 4c37930d44e529176a9a48c1b069a6f95206995a
Author: Martyn Russell <martyn lanedo com>
Date:   Fri Feb 21 21:43:02 2014 +0000

    extract: Refactor media_art_process_file() to simplify the function

 libmediaart/extract.c |  137 +++++++++++++++++++++++++++++++------------------
 libmediaart/extract.h |    2 +-
 2 files changed, 88 insertions(+), 51 deletions(-)
---
diff --git a/libmediaart/extract.c b/libmediaart/extract.c
index 5bd3c36..3d6e0ea 100644
--- a/libmediaart/extract.c
+++ b/libmediaart/extract.c
@@ -1297,6 +1297,37 @@ get_mtime (GFile   *file,
        return mtime;
 }
 
+gchar *
+get_heuristic_for_parent_path (GFile        *file,
+                               MediaArtType  type,
+                               const gchar  *artist,
+                               const gchar  *title)
+{
+       gchar *key;
+       gchar *parent_path = NULL;
+       GFile *parent;
+
+       if (!file) {
+               return NULL;
+       }
+
+       parent = g_file_get_parent (file);
+       if (parent) {
+               parent_path = g_file_get_path (parent);
+               g_object_unref (parent);
+       }
+
+       key = g_strdup_printf ("%i-%s-%s-%s",
+                              type,
+                              artist ? artist : "",
+                              title ? title : "",
+                              parent_path ? parent_path : "");
+
+       g_free (parent_path);
+
+       return key;
+}
+
 /**
  * media_art_process_file:
  * @file: File to be processed
@@ -1335,14 +1366,15 @@ media_art_process_file (GFile         *file,
 {
        GFile *cache_art_file, *local_art_file;
        GError *local_error = NULL;
-       gchar *art_path, *uri;
-       gchar *local_art_uri = NULL;
-       gboolean processed = TRUE, a_exists, created = FALSE;
-       guint64 mtime, a_mtime = 0;
+       gchar *cache_art_path, *uri;
+       gboolean processed, created, no_cache_or_old;
+       guint64 mtime, cache_mtime = 0;
 
        g_return_val_if_fail (G_IS_FILE (file), FALSE);
        g_return_val_if_fail (type > MEDIA_ART_NONE && type < MEDIA_ART_TYPE_COUNT, FALSE);
 
+       processed = created = FALSE;
+
        uri = g_file_get_uri (file);
        g_debug ("Processing media art: artist:'%s', title:'%s', type:'%s', uri:'%s'. Buffer is %ld bytes, 
mime:'%s'",
                 artist ? artist : "",
@@ -1354,7 +1386,7 @@ media_art_process_file (GFile         *file,
 
        mtime = get_mtime (file, &local_error);
        if (local_error != NULL) {
-               g_debug ("Could not get mtime for '%s': %s",
+               g_debug ("Could not get mtime for file '%s': %s",
                         uri,
                         local_error->message);
                g_propagate_error (error, local_error);
@@ -1370,59 +1402,62 @@ media_art_process_file (GFile         *file,
                            &cache_art_file,
                            &local_art_file);
 
-       if (!cache_art_file) {
-               g_debug ("Album art path could not be obtained, not processing any further");
+       cache_mtime = get_mtime (cache_art_file, &local_error);
+
+       if (local_error &&
+           local_error->domain == g_io_error_quark () &&
+           local_error->code == G_IO_ERROR_NOT_FOUND) {
+               /* cache_art_file not existing is the only error we
+                * accept here, anything else and we return.
+                */
+               gchar *path;
+
+               path = g_file_get_uri (cache_art_file);
+               g_debug ("Cache for media art didn't exist (%s)",
+                        path);
+               g_free (path);
+               g_clear_error (&local_error);
+       }
+
+       if (local_error) {
+               g_free (uri);
+
+               uri = g_file_get_uri (cache_art_file);
+               g_debug ("Could not get mtime for cache '%s': %s",
+                        uri,
+                        local_error->message);
+               g_free (uri);
 
                if (local_art_file) {
                        g_object_unref (local_art_file);
                }
 
-               g_free (uri);
+               g_propagate_error (error, local_error);
 
                return FALSE;
        }
 
-       a_exists = g_file_query_exists (cache_art_file, NULL);
-
-       if (a_exists) {
-               a_mtime = get_mtime (cache_art_file, &local_error);
-       }
-
-       art_path = g_file_get_path (cache_art_file);
-       local_art_uri = g_file_get_uri (local_art_file);
+       cache_art_path = g_file_get_path (cache_art_file);
+       no_cache_or_old = cache_mtime == 0 || mtime > cache_mtime;
 
-       if ((buffer && len > 0) && ((!a_exists) || (a_exists && mtime > a_mtime))) {
+       if ((buffer && len > 0) && no_cache_or_old) {
                processed = media_art_set (buffer, len, mime, type, artist, title);
-               set_mtime (art_path, mtime);
+               set_mtime (cache_art_path, mtime);
                created = TRUE;
        }
 
-       if ((!created) && ((!a_exists) || (a_exists && mtime > a_mtime))) {
+       if (!created && no_cache_or_old) {
                /* If not, we perform a heuristic on the dir */
                gchar *key;
-               gchar *dirname = NULL;
-               GFile *dirf;
 
-               dirf = g_file_get_parent (file);
-               if (dirf) {
-                       dirname = g_file_get_path (dirf);
-                       g_object_unref (dirf);
-               }
+               key = get_heuristic_for_parent_path (file, type, artist, title);
 
-               key = g_strdup_printf ("%i-%s-%s-%s",
-                                      type,
-                                      artist ? artist : "",
-                                      title ? title : "",
-                                      dirname ? dirname : "");
+               if (!g_hash_table_lookup (media_art_cache, key)) {
+                       gchar *local_art_uri;
 
-               g_free (dirname);
+                       local_art_uri = g_file_get_uri (local_art_file);
 
-               if (!g_hash_table_lookup (media_art_cache, key)) {
-                       if (!media_art_heuristic (artist,
-                                                 title,
-                                                 type,
-                                                 uri,
-                                                 local_art_uri)) {
+                       if (!media_art_heuristic (artist, title, type, uri, local_art_uri)) {
                                /* If the heuristic failed, we
                                 * request the download the
                                 * media-art to the media-art
@@ -1432,23 +1467,22 @@ media_art_process_file (GFile         *file,
                                                            artist,
                                                            title,
                                                            local_art_uri,
-                                                           art_path);
+                                                           cache_art_path);
                        }
 
-                       set_mtime (art_path, mtime);
+                       set_mtime (cache_art_path, mtime);
 
                        g_hash_table_insert (media_art_cache,
                                             key,
                                             GINT_TO_POINTER(TRUE));
+                       g_free (local_art_uri);
                } else {
                        g_free (key);
                }
-       } else {
-               if (!created) {
-                       g_debug ("Album art already exists for uri:'%s' as '%s'",
-                                uri,
-                                art_path);
-               }
+       } else if (!created) {
+               g_debug ("Album art already exists for uri:'%s' as '%s'",
+                        uri,
+                        cache_art_path);
        }
 
        if (local_art_file && !g_file_query_exists (local_art_file, NULL)) {
@@ -1456,7 +1490,11 @@ media_art_process_file (GFile         *file,
                 * situation might have changed
                 */
                if (g_file_query_exists (cache_art_file, NULL)) {
-                       media_art_copy_to_local (art_path, local_art_uri);
+                       gchar *local_art_uri;
+
+                       local_art_uri = g_file_get_uri (local_art_file);
+                       media_art_copy_to_local (cache_art_path, local_art_uri);
+                       g_free (local_art_uri);
                }
        }
 
@@ -1468,8 +1506,7 @@ media_art_process_file (GFile         *file,
                g_object_unref (local_art_file);
        }
 
-       g_free (art_path);
-       g_free (local_art_uri);
+       g_free (cache_art_path);
        g_free (uri);
 
        return processed;
@@ -1516,7 +1553,7 @@ media_art_process (const gchar          *uri,
                                         mime,
                                         type,
                                         artist,
-                                        title, 
+                                        title,
                                         error);
 
        g_object_unref (file);
diff --git a/libmediaart/extract.h b/libmediaart/extract.h
index c3063a0..da71fff 100644
--- a/libmediaart/extract.h
+++ b/libmediaart/extract.h
@@ -53,7 +53,7 @@ typedef enum {
  * @MEDIA_ART_ERROR_NOENT: The resource that the was passed (for example a
  * file or URI) does not exist.
  * @MEDIA_ART_ERROR_NOENT: The URI or GFile provided
- * points to a file that does not exist. 
+ * points to a file that does not exist.
  *
  * Enumeration values used in errors returned by the
  * #MediaArtError API.