[devhelp] BookManager: do not call g_file_test(exists) to find index file

[Sébastien Wilmet <swilmet src gnome org>]

commit 16283194b8ef16d2de24bf5f2a5291003617be27
Author: Sébastien Wilmet <swilmet gnome org>
Date:   Mon Dec 25 20:32:46 2017 +0100

    BookManager: do not call g_file_test(exists) to find index file
    
    As the documentation of g_file_query_exists() explains, it is better to
    try to open the file directly and handle G_IO_ERROR_NOT_FOUND
    gracefully. It is not racy that way, and is potentially faster since in
    most cases there will be one less I/O operation (if it finds directly a
    *.devhelp2 index file).

 src/dh-book-manager.c |  113 ++++++++++++++++++-------------------------------
 src/dh-book.c         |   10 ++++-
 src/dh-parser.c       |   17 ++++---
 3 files changed, 59 insertions(+), 81 deletions(-)
---
diff --git a/src/dh-book-manager.c b/src/dh-book-manager.c
index 1385b0b..bdb742b 100644
--- a/src/dh-book-manager.c
+++ b/src/dh-book-manager.c
@@ -22,10 +22,11 @@
 
 #include "config.h"
 #include "dh-book-manager.h"
-#include "dh-link.h"
 #include "dh-book.h"
 #include "dh-language.h"
+#include "dh-link.h"
 #include "dh-settings.h"
+#include "dh-util.h"
 
 /**
  * SECTION:dh-book-manager
@@ -80,8 +81,8 @@ static DhBookManager *singleton = NULL;
 
 G_DEFINE_TYPE_WITH_PRIVATE (DhBookManager, dh_book_manager, G_TYPE_OBJECT);
 
-static void create_book (DhBookManager *book_manager,
-                         GFile         *index_file);
+static gboolean create_book_from_index_file (DhBookManager *book_manager,
+                                             GFile         *index_file);
 
 static void
 dh_book_manager_get_property (GObject    *object,
@@ -271,35 +272,6 @@ dh_book_manager_class_init (DhBookManagerClass *klass)
                                                                G_PARAM_STATIC_STRINGS));
 }
 
-static gchar *
-get_book_path (const gchar *base_path,
-               const gchar *name)
-{
-        static const gchar *suffixes[] = {
-                "devhelp2",
-                "devhelp2.gz",
-                "devhelp",
-                "devhelp.gz",
-                NULL
-        };
-        gchar *tmp;
-        gchar *book_path;
-        guint i;
-
-        for (i = 0; suffixes[i]; i++) {
-                tmp = g_build_filename (base_path, name, NULL);
-                book_path = g_strconcat (tmp, ".", suffixes[i], NULL);
-                g_free (tmp);
-
-                if (g_file_test (book_path, G_FILE_TEST_EXISTS)) {
-                        return book_path;
-                }
-                g_free (book_path);
-        }
-
-        return NULL;
-}
-
 static void
 load_books_disabled (DhBookManager *book_manager)
 {
@@ -465,7 +437,7 @@ book_updated_cb (DhBook   *book,
 
         book_deleted_cb (book, book_manager);
 
-        create_book (book_manager, index_file);
+        create_book_from_index_file (book_manager, index_file);
         g_object_unref (index_file);
 }
 
@@ -534,9 +506,12 @@ book_disabled_cb (DhBook   *book,
                        book);
 }
 
-static void
-create_book (DhBookManager *book_manager,
-             GFile         *index_file)
+/* Returns TRUE if "successful", FALSE if the next possible index file in the
+ * book directory needs to be tried.
+ */
+static gboolean
+create_book_from_index_file (DhBookManager *book_manager,
+                             GFile         *index_file)
 {
         DhBookManagerPrivate *priv;
         DhBook *book;
@@ -553,12 +528,12 @@ create_book (DhBookManager *book_manager,
                 cur_index_file = dh_book_get_index_file (cur_book);
 
                 if (g_file_equal (index_file, cur_index_file))
-                        return;
+                        return TRUE;
         }
 
         book = dh_book_new (index_file);
         if (book == NULL)
-                return;
+                return FALSE;
 
         /* Check if book with same ID was already loaded in the manager (we need
          * to force unique book IDs).
@@ -567,7 +542,7 @@ create_book (DhBookManager *book_manager,
                                 book,
                                 (GCompareFunc)dh_book_cmp_by_id)) {
                 g_object_unref (book);
-                return;
+                return TRUE;
         }
 
         priv->books = g_list_insert_sorted (priv->books,
@@ -608,37 +583,39 @@ create_book (DhBookManager *book_manager,
                        signals[BOOK_CREATED],
                        0,
                        book);
+
+        return TRUE;
 }
 
-static gboolean
-new_possible_book_cb (gpointer user_data)
+static void
+create_book_from_directory (DhBookManager *book_manager,
+                            GFile         *book_directory)
 {
-        NewPossibleBookData *data = user_data;
-        gchar *file_path;
-        gchar *file_basename;
-        gchar *book_path;
-
-        file_path = g_file_get_path (data->file);
-        file_basename = g_file_get_basename (data->file);
+        GSList *possible_index_files;
+        GSList *l;
 
-        /* Compute book path, will return NULL if it's not a proper path */
-        book_path = get_book_path (file_path, file_basename);
-        if (book_path != NULL) {
-                GFile *index_file;
+        possible_index_files = dh_util_get_possible_index_files (book_directory);
 
-                index_file = g_file_new_for_path (book_path);
-                create_book (data->book_manager, index_file);
-                g_object_unref (index_file);
+        for (l = possible_index_files; l != NULL; l = l->next) {
+                GFile *index_file = G_FILE (l->data);
 
-                g_free (book_path);
+                if (create_book_from_index_file (book_manager, index_file))
+                        break;
         }
 
-        g_free (file_path);
-        g_free (file_basename);
+        g_slist_free_full (possible_index_files, g_object_unref);
+}
+
+static gboolean
+new_possible_book_cb (gpointer user_data)
+{
+        NewPossibleBookData *data = user_data;
+
+        create_book_from_directory (data->book_manager, data->file);
+
         g_object_unref (data->book_manager);
         g_object_unref (data->file);
         g_slice_free (NewPossibleBookData, data);
-
         return G_SOURCE_REMOVE;
 }
 
@@ -742,25 +719,17 @@ add_books_in_dir (DhBookManager *book_manager,
         /* And iterate it */
         while ((name = g_dir_read_name (dir)) != NULL) {
                 gchar *book_dir_path;
-                gchar *book_path;
+                GFile *book_directory;
 
                 /* Build the path of the directory where the final
                  * devhelp book resides */
-                book_dir_path = g_build_filename (dir_path,
-                                                  name,
-                                                  NULL);
+                book_dir_path = g_build_filename (dir_path, name, NULL);
+                book_directory = g_file_new_for_path (book_dir_path);
 
-                book_path = get_book_path (book_dir_path, name);
-                if (book_path != NULL) {
-                        GFile *index_file;
+                create_book_from_directory (book_manager, book_directory);
 
-                        index_file = g_file_new_for_path (book_path);
-                        create_book (book_manager, index_file);
-                        g_object_unref (index_file);
-
-                        g_free (book_path);
-                }
                 g_free (book_dir_path);
+                g_object_unref (book_directory);
         }
 
         g_dir_close (dir);
diff --git a/src/dh-book.c b/src/dh-book.c
index b838f82..1b45072 100644
--- a/src/dh-book.c
+++ b/src/dh-book.c
@@ -313,7 +313,12 @@ dh_book_new (GFile *index_file)
                                   &priv->tree,
                                   &priv->links,
                                   &error)) {
-                if (error != NULL) {
+                /* It's fine if the file doesn't exist, as DhBookManager tries
+                 * to create a DhBook for each possible index file in a certain
+                 * book directory.
+                 */
+                if (error != NULL &&
+                    !g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) {
                         gchar *parse_name;
 
                         parse_name = g_file_get_parse_name (priv->index_file);
@@ -323,9 +328,10 @@ dh_book_new (GFile *index_file)
                                    error->message);
 
                         g_free (parse_name);
-                        g_clear_error (&error);
                 }
 
+                g_clear_error (&error);
+
                 /* Deallocate the book, as we are not going to add it in the
                  * manager.
                  */
diff --git a/src/dh-parser.c b/src/dh-parser.c
index 65ecb2a..744cd69 100644
--- a/src/dh-parser.c
+++ b/src/dh-parser.c
@@ -569,13 +569,6 @@ dh_parser_read_file (GFile   *index_file,
                 gz = TRUE;
         }
 
-        if (parser->version == FORMAT_VERSION_1)
-                g_warning ("The file '%s' uses the Devhelp index file format version 1, "
-                           "which is deprecated. A future version of Devhelp may remove "
-                           "the support for the format version 1. The index file should "
-                           "be ported to the Devhelp index file format version 2.",
-                           index_file_uri);
-
         parser->markup_parser = g_new0 (GMarkupParser, 1);
         parser->markup_parser->start_element = parser_start_node_cb;
         parser->markup_parser->end_element = parser_end_node_cb;
@@ -590,6 +583,16 @@ dh_parser_read_file (GFile   *index_file,
                 goto exit;
         }
 
+        /* At this point we know that the file exists, the G_IO_ERROR_NOT_FOUND
+         * has been catched earlier. So print warning.
+         */
+        if (parser->version == FORMAT_VERSION_1)
+                g_warning ("The file '%s' uses the Devhelp index file format version 1, "
+                           "which is deprecated. A future version of Devhelp may remove "
+                           "the support for the format version 1. The index file should "
+                           "be ported to the Devhelp index file format version 2.",
+                           index_file_uri);
+
         if (gz) {
                 GZlibDecompressor *zlib_decompressor;