From 45d47de17d19dd55ea9361d8a33caadc633f0fed Mon Sep 17 00:00:00 2001 From: Martyn Russell Date: Wed, 10 Sep 2014 11:26:11 +0100 Subject: cache: Added media_art_remove_async() and _finish() Part of this API changes makes the media_art_get_{path|file}() APIs not do i/o operations like creating the cache directory. This is now done in media_art_process_new(). https://bugzilla.gnome.org/show_bug.cgi?id=724879 --- .../reference/libmediaart/libmediaart-sections.txt | 2 + libmediaart/cache.c | 167 +++++++++++++++++++-- libmediaart/cache.h | 15 +- libmediaart/extract.c | 32 +++- libmediaart/extract.h | 9 +- tests/mediaarttest.c | 61 ++++++-- 6 files changed, 251 insertions(+), 35 deletions(-) diff --git a/docs/reference/libmediaart/libmediaart-sections.txt b/docs/reference/libmediaart/libmediaart-sections.txt index 77bd1f7..1de8fe7 100644 --- a/docs/reference/libmediaart/libmediaart-sections.txt +++ b/docs/reference/libmediaart/libmediaart-sections.txt @@ -3,6 +3,8 @@ media_art_get_path media_art_get_file media_art_remove +media_art_remove_async +media_art_remove_finish media_art_strip_invalid_entities diff --git a/libmediaart/cache.c b/libmediaart/cache.c index 3492683..a145931 100644 --- a/libmediaart/cache.c +++ b/libmediaart/cache.c @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -271,6 +272,9 @@ media_art_checksum_for_data (GChecksumType checksum_type, * When done, both #GFiles must be freed with g_object_unref() if * non-%NULL. * + * This operation should not use i/o, but it depends on the backend + * GFile implementation. + * * Returns: %TRUE if @cache_file or @local_file were returned, otherwise %FALSE. * * Since: 0.2.0 @@ -332,10 +336,6 @@ media_art_get_file (const gchar *artist, "media-art", NULL); - if (!g_file_test (dir, G_FILE_TEST_EXISTS)) { - g_mkdir_with_parents (dir, 0770); - } - if (artist) { a = artist_checksum; b = title ? title_checksum : space_checksum; @@ -470,18 +470,22 @@ media_art_remove_foreach (gpointer data, * media_art_remove: * @artist: artist the media art belongs to * @album: (allow-none): album the media art belongs or %NULL + * @cancellable: (allow-none): optional #GCancellable object, %NULL to ignore. + * @error: location to store the error occurring, or %NULL to ignore * * Removes media art for given album/artist provided. * - * Returns: #TRUE on success, otherwise #FALSE. + * Returns: #TRUE on success, otherwise #FALSE where @error will be set. * * Since: 0.2.0 */ gboolean -media_art_remove (const gchar *artist, - const gchar *album) +media_art_remove (const gchar *artist, + const gchar *album, + GCancellable *cancellable, + GError **error) { - GError *error = NULL; + GError *local_error = NULL; GHashTable *table = NULL; const gchar *name; GDir *dir; @@ -494,13 +498,14 @@ media_art_remove (const gchar *artist, dirname = g_build_filename (g_get_user_cache_dir (), "media-art", NULL); - dir = g_dir_open (dirname, 0, &error); - if (!dir || error) { + dir = g_dir_open (dirname, 0, &local_error); + if (!dir || local_error) { /* Nothing to do if there is no directory in the first place. */ g_debug ("Removing media-art for artist:'%s', album:'%s': directory could not be opened, %s", - artist, album, error ? error->message : "no error given"); + artist, album, local_error ? local_error->message : "no error given"); + + g_clear_error (&local_error); - g_clear_error (&error); if (dir) { g_dir_close (dir); } @@ -553,6 +558,13 @@ media_art_remove (const gchar *artist, g_list_foreach (to_remove, media_art_remove_foreach, &success); g_list_free (to_remove); + if (!success) { + g_set_error_literal (error, + G_IO_ERROR, + G_IO_ERROR_FAILED, + _("Could not remove one or more files from media art cache")); + } + g_hash_table_unref (table); g_dir_close (dir); @@ -560,3 +572,134 @@ media_art_remove (const gchar *artist, return success; } + +typedef struct { + gchar *artist; + gchar *album; +} RemoveData; + +static RemoveData * +remove_data_new (const gchar *artist, + const gchar *album) +{ + RemoveData *data; + + data = g_slice_new0 (RemoveData); + data->artist = g_strdup (artist); + data->album = g_strdup (album); + + return data; +} + +static void +remove_data_free (RemoveData *data) +{ + if (!data) { + return; + } + + g_free (data->artist); + g_free (data->album); + g_slice_free (RemoveData, data); +} + +static void +remove_thread (GTask *task, + gpointer source_object, + gpointer task_data, + GCancellable *cancellable) +{ + RemoveData *data = task_data; + GError *error = NULL; + gboolean success = FALSE; + + if (!g_cancellable_set_error_if_cancelled (cancellable, &error)) { + success = media_art_remove (data->artist, + data->album, + cancellable, + &error); + } + + if (error) { + g_task_return_error (task, error); + } else { + g_task_return_boolean (task, success); + } +} + +/** + * media_art_remove_async: + * @artist: artist the media art belongs to + * @album: (allow-none): album the media art belongs or %NULL + * @source_object: (allow-none): the #GObject this task belongs to, + * can be %NULL. + * @io_priority: the [I/O priority][io-priority] of the request + * @cancellable: (allow-none): optional #GCancellable object, %NULL to + * ignore + * @callback: (scope async): a #GAsyncReadyCallback to call when the + * request is satisfied + * @user_data: (closure): the data to pass to callback function + * + * Removes media art for given album/artist provided. Precisely the + * same operation as media_art_remove() is performing, but + * asynchronously. + * + * When all i/o for the operation is finished the @callback will be + * called. + * + * In case of a partial error the callback will be called with any + * succeeding items and no error, and on the next request the error + * will be reported. If a request is cancelled the callback will be + * called with %G_IO_ERROR_CANCELLED. + * + * During an async request no other sync and async calls are allowed, + * and will result in %G_IO_ERROR_PENDING errors. + * + * Any outstanding i/o request with higher priority (lower numerical + * value) will be executed before an outstanding request with lower + * priority. Default priority is %G_PRIORITY_DEFAULT. + * + * Since: 0.7.0 + */ +void +media_art_remove_async (const gchar *artist, + const gchar *album, + gint io_priority, + GObject *source_object, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data) +{ + GTask *task; + + task = g_task_new (source_object, cancellable, callback, user_data); + g_task_set_task_data (task, remove_data_new (artist, album), (GDestroyNotify) remove_data_free); + g_task_set_priority (task, io_priority); + g_task_run_in_thread (task, remove_thread); + g_object_unref (task); +} + +/** + * media_art_remove_finish: + * @source_object: (allow-none): the #GObject this task belongs to, + * can be %NULL. + * @result: a #GAsyncResult. + * @error: a #GError location to store the error occurring, or %NULL + * to ignore. + * + * Finishes the asynchronous operation started with + * media_art_remove_async(). + * + * Returns: %TRUE on success, otherwise %FALSE when @error will be set. + * + * Since: 0.7.0 + **/ +gboolean +media_art_remove_finish (GObject *source_object, + GAsyncResult *result, + GError **error) +{ + g_return_val_if_fail (g_task_is_valid (result, source_object), FALSE); + + return g_task_propagate_boolean (G_TASK (result), error); +} diff --git a/libmediaart/cache.h b/libmediaart/cache.h index e5bf6b8..be18e04 100644 --- a/libmediaart/cache.h +++ b/libmediaart/cache.h @@ -37,7 +37,6 @@ gboolean media_art_get_path (const gchar *artist, const gchar *uri, gchar **cache_path, gchar **local_uri); - gboolean media_art_get_file (const gchar *artist, const gchar *title, const gchar *prefix, @@ -46,7 +45,19 @@ gboolean media_art_get_file (const gchar *artist, GFile **local_file); gboolean media_art_remove (const gchar *artist, - const gchar *album); + const gchar *album, + GCancellable *cancellable, + GError **error); +void media_art_remove_async (const gchar *artist, + const gchar *album, + gint io_priority, + GObject *source_object, + GCancellable *cancellable, + GAsyncReadyCallback callback, + gpointer user_data); +gboolean media_art_remove_finish (GObject *source_object, + GAsyncResult *result, + GError **error); G_END_DECLS diff --git a/libmediaart/extract.c b/libmediaart/extract.c index 692a3ee..faaf994 100644 --- a/libmediaart/extract.c +++ b/libmediaart/extract.c @@ -24,6 +24,7 @@ #include #include +#include #include #include "extractgeneric.h" @@ -146,6 +147,8 @@ media_art_process_initable_init (GInitable *initable, MediaArtProcessPrivate *private; MediaArtProcess *process; GError *local_error = NULL; + gchar *dir; + gboolean retval; process = MEDIA_ART_PROCESS (initable); private = media_art_process_get_instance_private (process); @@ -174,17 +177,32 @@ media_art_process_initable_init (GInitable *initable, private->storage = storage_new (); if (!private->storage) { g_critical ("Could not start storage module for removable media detection"); + g_set_error_literal (error, + media_art_error_quark (), + MEDIA_ART_ERROR_NO_STORAGE, + _("Could not initialize storage module")); + return FALSE; + } - if (error) { - *error = g_error_new (media_art_error_quark (), - MEDIA_ART_ERROR_NO_STORAGE, - "Could not initialize storage module"); - } + /* Returns 0 if already exists, so we don't check if directory + * existed before, it's an additional stat() call we just + * don't need. + */ + dir = g_build_filename (g_get_user_cache_dir (), "media-art", NULL); + retval = g_mkdir_with_parents (dir, 0770); - return FALSE; + if (retval == -1) { + g_set_error (error, + media_art_error_quark (), + MEDIA_ART_ERROR_NO_CACHE_DIR, + _("Could not create cache directory '%s', %d returned by g_mkdir_with_parents()"), + dir, + retval); } - return TRUE; + g_free (dir); + + return retval == 0 ? TRUE : FALSE; } static void diff --git a/libmediaart/extract.h b/libmediaart/extract.h index e5ab8fa..2356790 100644 --- a/libmediaart/extract.h +++ b/libmediaart/extract.h @@ -69,6 +69,10 @@ typedef enum { * @MEDIA_ART_ERROR_SYMLINK_FAILED: A call to symlink() failed * resulting in the incorrect storage of media art. * @MEDIA_ART_ERROR_RENAME_FAILED: File could not be renamed. + * @MEDIA_ART_ERROR_NO_CACHE_DIR: This is given when the + * XDG_CACHE_HOME directory could not be used to create the + * 'media-art' subdirectory used for caching media art. This is + * usually an initiation error. * * Enumeration values used in errors returned by the * #MediaArtError API. @@ -79,14 +83,15 @@ typedef enum { MEDIA_ART_ERROR_NO_STORAGE, MEDIA_ART_ERROR_NO_TITLE, MEDIA_ART_ERROR_SYMLINK_FAILED, - MEDIA_ART_ERROR_RENAME_FAILED + MEDIA_ART_ERROR_RENAME_FAILED, + MEDIA_ART_ERROR_NO_CACHE_DIR } MediaArtError; /** * media_art_error_quark: * - * A #GQuark representing the type of #GError for #MediaArtProcess failures. + * Returns: A #GQuark representing the type of #GError for #MediaArtProcess failures. * * Since: 0.2.0 **/ diff --git a/tests/mediaarttest.c b/tests/mediaarttest.c index df34497..22694b6 100644 --- a/tests/mediaarttest.c +++ b/tests/mediaarttest.c @@ -23,6 +23,7 @@ #include #include +#include #include @@ -238,12 +239,34 @@ test_mediaart_embedded_mp3 (void) g_object_unref (process); } +static void +test_mediaart_remove_cb (GObject *source_object, + GAsyncResult *result, + gpointer user_data) +{ + GError *error = NULL; + GFile *file = user_data; + gboolean success; + + success = media_art_remove_finish (source_object, result, &error); + g_assert_no_error (error); + g_assert_true (success); + + success = media_art_remove ("Lanedo", NULL, NULL, &error); + g_assert_no_error (error); + g_assert_true (success); + + g_object_unref (file); +} + static void test_mediaart_process_buffer (void) { MediaArtProcess *process; + GCancellable *cancellable; GError *error = NULL; GFile *file = NULL; + gchar *dir; gchar *path; gchar *out_path = NULL; gchar *out_uri = NULL; @@ -253,9 +276,6 @@ test_mediaart_process_buffer (void) path = g_build_filename (G_DIR_SEPARATOR_S, TOP_SRCDIR, "tests", "cover.png", NULL); file = g_file_new_for_path (path); - process = media_art_process_new (&error); - g_assert_no_error (error); - /* Check data is not cached currently */ media_art_get_path ("Lanedo", /* artist / title */ NULL, /* album */ @@ -263,10 +283,18 @@ test_mediaart_process_buffer (void) path, &out_path, &out_uri); - g_assert (g_file_test (out_path, G_FILE_TEST_EXISTS) == FALSE); + g_assert_false (g_file_test (out_path, G_FILE_TEST_EXISTS)); g_free (out_path); g_free (out_uri); + /* Creates media-art cache dir if it doesn't exist ... */ + process = media_art_process_new (&error); + g_assert_no_error (error); + + dir = g_build_filename (g_get_user_cache_dir (), "media-art", NULL); + g_assert_true (g_file_test (dir, G_FILE_TEST_EXISTS)); + g_free (dir); + /* Process data */ retval = media_art_process_file (process, MEDIA_ART_ALBUM, @@ -303,14 +331,17 @@ test_mediaart_process_buffer (void) g_free (expected); /* Remove album art */ - retval = media_art_remove ("Lanedo", ""); - g_assert_true (retval); - - retval = media_art_remove ("Lanedo", NULL); - - g_object_unref (file); + cancellable = g_cancellable_new (); + media_art_remove_async ("Lanedo", + "", + G_PRIORITY_DEFAULT, + G_OBJECT (process), + cancellable, + test_mediaart_remove_cb, + file); + + g_object_unref (cancellable); g_free (path); - g_object_unref (process); } @@ -381,7 +412,8 @@ main (int argc, char **argv) { const gchar *cache_home_originally; const gchar *test_dir; - gint success = EXIT_SUCCESS; + gchar *dir; + gint success; gint i; g_test_init (&argc, &argv, NULL); @@ -420,6 +452,11 @@ main (int argc, char **argv) success = g_test_run (); + /* Clean up */ + dir = g_build_filename (g_get_user_cache_dir (), "media-art", NULL); + g_rmdir (dir); + g_free (dir); + if (cache_home_originally) { g_setenv ("XDG_CACHE_HOME", cache_home_originally, TRUE); } else { -- cgit v1.2.1