[gnome-autoar/wip/razvan/extract-refactor2: 2/3] AutoarExtract: refactor destination decision process

[Răzvan-Mihai Chițu <razvanchitu src gnome org>]

commit 8f9dfd82d8df06dbaf58233928ce6a3b1d8e2f40
Author: Razvan Chitu <razvan ch95 gmail com>
Date:   Sat May 28 22:59:00 2016 +0300

    AutoarExtract: refactor destination decision process
    
    Previously, the destination of an extraction could not be modified by the user
    application once the AutoarExtract object was created. This leads to a big lack
    of flexibility, because there are many cases when the destination depends on the
    contents of the archive, unknown at object creation time.
    
    In order to fix this, when the destination is notified to the user, also notify
    the list of files in the archive and enable the user to set a new destination.

 gnome-autoar/autoar-extract.c |  220 +++++++++++++++++++++++++++--------------
 tests/test-extract.c          |   24 ++++-
 2 files changed, 166 insertions(+), 78 deletions(-)
---
diff --git a/gnome-autoar/autoar-extract.c b/gnome-autoar/autoar-extract.c
index 4572eb5..36ffadd 100644
--- a/gnome-autoar/autoar-extract.c
+++ b/gnome-autoar/autoar-extract.c
@@ -59,20 +59,29 @@
  * @Include: gnome-autoar/autoar.h
  *
  * The #AutoarExtract object is used to automatically extract files and
- * directories from an archive. It will create a file in the output directory
- * if the archive only contains one file, or it will create a directory in the
- * output directory if the archive contains more than one file. #AutoarExtract
- * create only one file or directory in the output directory, so the output
- * directory will not be messed. All absolute paths in archives will be
- * converted to relative paths and all extracted files will be placed under the
- * directory notified by #AutoarExtract::decide-dest. The created file
- * or directory name will be generated based on the name of the source archive,
- * so users can easily figure out the relation between the archive and the
- * extracted files. #AutoarExtract can also ignore specific file name pattern
- * when extrating, or delete the source archive after extracting, depending on
- * the settings provided by the #AutoarPref object.
+ * directories from an archive. By default, it will only create one file or
+ * directory in the output directory. In this way, the output directory will
+ * not get messed. If the archive contains only one file, the file will be
+ * extracted to the output directory. If the archive has more files, they will
+ * be put in a directory having the same name as the archive, minus the
+ * extension. It is also possible to extract all files to a specified directory
+ * without any checks by using autoar_extract_set_output_is_dest().
+
+ * #AutoarExtract will not attempt to solve any name conflicts. If the
+ * destination directory already exists, it will proceed normally. If the
+ * destionation directory cannot be created, it will fail with an error.
+ * It is possible however to change the destination, when
+ * #AutoarExtract::confirm-dest gets emitted. The signal notifies the decided
+ * destination and the list of files to be extracted. The signal also allows a
+ * new destination to be set, which will override the previous one. This is
+ * convenient for solving name conflicts and implementing specific logic based
+ * on the contents of the archive.
+
+ * #AutoarExtract can also ignore specific file name pattern when extrating,
+ * or delete the source archive after extracting, depending on the settings
+ * provided by the #AutoarPref object.
  *
- * When #AutoarExtract stop all work, it will emit one of the three signals:
+ * When #AutoarExtract stops all work, it will emit one of the three signals:
  * #AutoarExtract::cancelled, #AutoarExtract::error, and
  * #AutoarExtract::completed. After one of these signals is received,
  * the #AutoarExtract object should be destroyed because it cannot be used to
@@ -125,8 +134,6 @@ struct _AutoarExtractPrivate
   guint64 size;
   guint64 completed_size;
 
-  GList *files_list;
-
   guint files;
   guint completed_files;
 
@@ -138,12 +145,14 @@ struct _AutoarExtractPrivate
   gssize        buffer_size;
   GError       *error;
 
+  GList *files_list;
+
   GHashTable *userhash;
   GHashTable *grouphash;
   GHashTable *bad_filename;
   GPtrArray  *pattern_compiled;
   GArray     *extracted_dir_list;
-  GFile      *top_level_dir;
+  GFile      *destination_dir;
 
   GFile *prefix;
 
@@ -162,7 +171,7 @@ struct _GFileAndInfo
 enum
 {
   SCANNED,
-  DECIDE_DEST,
+  CONFIRM_DESTINATION,
   PROGRESS,
   CANCELLED,
   COMPLETED,
@@ -467,17 +476,18 @@ autoar_extract_get_notify_interval (AutoarExtract *arextract)
  * has been already decided
  *
  * By default #AutoarExtract:output-is-dest is set to %FALSE, which means
- * the extracted files will be created as a single file or a single top-level
- * directory under #AutoarExtract:output directory. The name of the extracted
- * file or directory will be automatically generated and you will be notified
- * via #AutoarExtract::decide-dest when the name is decided. If you have
- * already decided the location of the extracted file or directory, and you
- * do not want #AutoarExtract to decide it for you, you can set
- * #AutoarExtract:output-is-dest to %TRUE. #AutoarExtract will use
- * #AutoarExtract:output as the location of the extracted directory or file,
- * and it will neither check whether the file exists nor create the necessary
- * directories for you. This function should only be called before calling
- * autoar_extract_start() or autoar_extract_start_async().
+ * only one file or directory will be generated. The destination is internally
+ * determined by analyzing the contents of the archive. If this is not wanted,
+ * #AutoarExtract:output-is-dest can be set to %TRUE, which will make
+ * #AutoarExtract:output the destination for extracted files. In any case, the
+ * destination directory will be notified via #AutoarExtract::confirm-dest,
+ * when it is possible to set a new destination.
+ *
+ * #AutoarExtract will attempt to create the destination directory regardless
+ * to whether its path was internally decided or not.
+
+ * This function should only be called before calling autoar_extract_start() or
+ * autoar_extract_start_async().
  **/
 void
 autoar_extract_set_output_is_dest  (AutoarExtract *arextract,
@@ -528,7 +538,7 @@ autoar_extract_dispose (GObject *object)
   g_clear_object (&(priv->source_file));
   g_clear_object (&(priv->output_file));
   g_clear_object (&(priv->arpref));
-  g_clear_object (&(priv->top_level_dir));
+  g_clear_object (&(priv->destination_dir));
   g_clear_object (&(priv->cancellable));
   g_clear_object (&(priv->prefix));
 
@@ -803,11 +813,15 @@ autoar_extract_signal_scanned (AutoarExtract *arextract)
 }
 
 static inline void
-autoar_extract_signal_decide_dest (AutoarExtract *arextract)
+autoar_extract_signal_confirm_dest (AutoarExtract *arextract,
+                                    GList *files,
+                                    GFile **new_destination)
 {
   autoar_common_g_signal_emit (arextract, arextract->priv->in_thread,
-                               autoar_extract_signals[DECIDE_DEST], 0,
-                               arextract->priv->top_level_dir);
+                               autoar_extract_signals[CONFIRM_DESTINATION], 0,
+                               arextract->priv->destination_dir,
+                               files,
+                               new_destination);
 }
 
 static inline void
@@ -887,19 +901,19 @@ autoar_extract_get_common_prefix (GList *files,
 
 static GFile*
 autoar_extract_do_sanitize_pathname (const char *pathname,
-                                     GFile *top_level_dir) {
+                                     GFile *destination_dir) {
   GFile *extracted_filename;
 
-  extracted_filename = g_file_get_child (top_level_dir, pathname);
+  extracted_filename = g_file_get_child (destination_dir, pathname);
 
-  if (!g_file_has_prefix (extracted_filename, top_level_dir)) {
+  if (!g_file_has_prefix (extracted_filename, destination_dir)) {
     char *basename;
 
     basename = g_file_get_basename (extracted_filename);
 
     g_object_unref (extracted_filename);
 
-    extracted_filename = g_file_get_child (top_level_dir, basename);
+    extracted_filename = g_file_get_child (destination_dir, basename);
 
     g_free (basename);
   }
@@ -1382,22 +1396,29 @@ autoar_extract_class_init (AutoarExtractClass *klass)
                   G_TYPE_UINT);
 
 /**
- * AutoarExtract::decide-dest:
+ * AutoarExtract::confirm-dest:
  * @arextract: the #AutoarExtract
- * @destination: the location of the extracted directory or file
+ * @destination: the location where files will be extracted
+ * @files: the list of files to be extracted. All have @destination as their
+           common prefix
  *
- * This signal is emitted when the location of the extracted directory or
- * file is determined.
+ * Returns: (transfer full): a new destination that will overwrite the previous
+ *                           one, or %NULL if this is not wanted
+ *
+ * This signal is emitted when the path of the destination directory is
+ * determined. It is useful for solving name conflicts or for setting a new
+ * destination, based on the contents of the archive.
  **/
-  autoar_extract_signals[DECIDE_DEST] =
-    g_signal_new ("decide-dest",
+  autoar_extract_signals[CONFIRM_DESTINATION] =
+    g_signal_new ("confirm-dest",
                   type,
                   G_SIGNAL_RUN_LAST,
                   0, NULL, NULL,
                   g_cclosure_marshal_generic,
-                  G_TYPE_NONE,
-                  1,
-                  G_TYPE_FILE);
+                  G_TYPE_OBJECT,
+                  2,
+                  G_TYPE_FILE,
+                  G_TYPE_POINTER);
 
 /**
  * AutoarExtract::progress:
@@ -1506,7 +1527,7 @@ autoar_extract_init (AutoarExtract *arextract)
   priv->pattern_compiled = g_ptr_array_new_with_free_func (g_pattern_spec_free_safe);
   priv->extracted_dir_list = g_array_new (FALSE, FALSE, sizeof (GFileAndInfo));
   g_array_set_clear_func (priv->extracted_dir_list, g_file_and_info_free);
-  priv->top_level_dir = NULL;
+  priv->destination_dir = NULL;
 
   priv->suggested_destname = NULL;
 
@@ -1824,9 +1845,9 @@ autoar_extract_step_scan_toplevel (AutoarExtract *arextract)
 }
 
 static void
-autoar_extract_step_decide_dest (AutoarExtract *arextract) {
-  /* Step 2: Create necessary directories
-   * If the archive contains only one file, we don't create the directory */
+autoar_extract_step_decide_dest (AutoarExtract *arextract)
+{
+  /* Step 2: Decide destination based on archive contents */
 
   AutoarExtractPrivate *priv;
 
@@ -1846,44 +1867,94 @@ autoar_extract_step_decide_dest (AutoarExtract *arextract) {
     prefix_name_no_ext = autoar_common_get_basename_remove_extension (prefix_name);
 
     if (g_strcmp0 (prefix_name_no_ext, priv->suggested_destname) == 0) {
-      priv->top_level_dir = g_object_ref (priv->output_file);
+      priv->destination_dir = g_object_ref (priv->output_file);
     }
 
     g_free (prefix_name);
     g_free (prefix_name_no_ext);
   }
-  /* If none of the above situations apply, the top level directory has the
-   * name of the source archive (minus the extension)
+  /* If none of the above situations apply, the top level directory gets the
+   * name suggested when creating the AutoarExtract object
    */
-  if (priv->top_level_dir == NULL) {
-    priv->top_level_dir = g_file_get_child (priv->output_file,
-                                            priv->suggested_destname);
+  if (priv->destination_dir == NULL) {
+    priv->destination_dir = g_file_get_child (priv->output_file,
+                                              priv->suggested_destname);
   }
+}
 
-  g_file_make_directory_with_parents (priv->top_level_dir, priv->cancellable,
-                                      &(priv->error));
+static void
+autoar_extract_step_decide_dest_already (AutoarExtract *arextract) {
+  /* Alternative step 2: Destination is output, so no decision has to be made */
 
-  if (g_error_matches (priv->error, G_IO_ERROR, G_IO_ERROR_EXISTS)) {
-    g_clear_error (&priv->error);
+  arextract->priv->destination_dir = g_object_ref (arextract->priv->output_file);
+}
+
+static void
+autoar_extract_step_confirm_dest (AutoarExtract *arextract)
+{
+  /* Step 3: Confirm destination */
+
+  AutoarExtractPrivate *priv;
+  GList *files = NULL;
+  GList *l;
+  GFile *new_destination = NULL;
+
+  priv = arextract->priv;
+
+  for (l = priv->files_list; l != NULL; l = l->next) {
+    char *relative_path;
+    GFile *file;
+
+    relative_path = g_file_get_relative_path (priv->output_file, l->data);
+    file = g_file_resolve_relative_path (priv->destination_dir,
+                                         relative_path);
+    files = g_list_prepend (files, file);
+
+    g_free (relative_path);
   }
 
-  if (priv->error != NULL) {
-    return;
+  files = g_list_reverse (files);
+
+  autoar_extract_signal_confirm_dest (arextract, files, &new_destination);
+
+  if (new_destination != NULL) {
+    g_object_unref (priv->destination_dir);
+    priv->destination_dir = new_destination;
+
+    g_debug ("autoar_extract_step_confirm_dest: destination changed");
   }
 
-  autoar_extract_signal_decide_dest (arextract);
-}
+  {
+    char *destination_name;
 
-static void
-autoar_extract_step_decide_dest_already (AutoarExtract *arextract) {
-  /* Alternative step 2: Output is destination */
-  arextract->priv->top_level_dir = g_object_ref (arextract->priv->output_file);
-  autoar_extract_signal_decide_dest (arextract);
+    destination_name = g_file_get_path (priv->destination_dir);
+    g_debug ("autoar_extract_step_confirm_dest: destination %s", destination_name);
+
+    g_free (destination_name);
+  }
+
+  g_file_make_directory_with_parents (priv->destination_dir, priv->cancellable,
+                                      &(priv->error));
+
+  if (g_error_matches (priv->error, G_IO_ERROR, G_IO_ERROR_EXISTS)) {
+    GFileType file_type;
+
+    file_type = g_file_query_file_type (priv->destination_dir,
+                                        G_FILE_QUERY_INFO_NONE,
+                                        NULL);
+
+    if (file_type == G_FILE_TYPE_DIRECTORY) {
+      g_debug ("autoar_extract_step_confirm_dest: destination directory exists");
+      g_clear_error (&priv->error);
+    }
+  }
+
+  g_list_free_full (files, g_object_unref);
 }
 
 static void
 autoar_extract_step_extract (AutoarExtract *arextract) {
-  /* Step 3: Extract files
+  /* Step 4: Extract files
    * We have to re-open the archive to extract files */
 
   struct archive *a;
@@ -1923,11 +1994,11 @@ autoar_extract_step_extract (AutoarExtract *arextract) {
       continue;
 
     extracted_filename =
-      autoar_extract_do_sanitize_pathname (pathname, priv->top_level_dir);
+      autoar_extract_do_sanitize_pathname (pathname, priv->destination_dir);
 
     if (hardlink != NULL) {
       hardlink_filename =
-        autoar_extract_do_sanitize_pathname (hardlink, priv->top_level_dir);
+        autoar_extract_do_sanitize_pathname (hardlink, priv->destination_dir);
     }
 
     autoar_extract_do_write_entry (arextract, a, entry,
@@ -1959,7 +2030,7 @@ autoar_extract_step_extract (AutoarExtract *arextract) {
 
 static void
 autoar_extract_step_apply_dir_fileinfo (AutoarExtract *arextract) {
-  /* Step 4: Re-apply file info to all directories
+  /* Step 5: Re-apply file info to all directories
    * It is required because modification times may be updated during the
    * writing of files in the directory. */
 
@@ -1984,7 +2055,7 @@ autoar_extract_step_apply_dir_fileinfo (AutoarExtract *arextract) {
 
 static void
 autoar_extract_step_cleanup (AutoarExtract *arextract) {
-  /* Step 5: Force progress to be 100% and remove the source archive file
+  /* Step 6: Force progress to be 100% and remove the source archive file
    * If the extraction is completed successfully, remove the source file.
    * Errors are not fatal because we have completed our work. */
 
@@ -2010,7 +2081,7 @@ autoar_extract_run (AutoarExtract *arextract)
 {
   /* Numbers of steps.
    * The array size must be modified if more steps are added. */
-  void (*steps[7])(AutoarExtract*);
+  void (*steps[8])(AutoarExtract*);
 
   AutoarExtractPrivate *priv;
   int i;
@@ -2033,6 +2104,7 @@ autoar_extract_run (AutoarExtract *arextract)
   steps[i++] = priv->output_is_dest ?
                autoar_extract_step_decide_dest_already :
                autoar_extract_step_decide_dest;
+  steps[i++] = autoar_extract_step_confirm_dest;
   steps[i++] = autoar_extract_step_extract;
   steps[i++] = autoar_extract_step_apply_dir_fileinfo;
   steps[i++] = autoar_extract_step_cleanup;
diff --git a/tests/test-extract.c b/tests/test-extract.c
index 197eca8..2603da7 100644
--- a/tests/test-extract.c
+++ b/tests/test-extract.c
@@ -13,17 +13,33 @@ my_handler_scanned (AutoarExtract *arextract,
   g_print ("Scanning OK, %d files to be extracted.\n", files);
 }
 
-static void
-my_handler_decide_dest (AutoarExtract *arextract,
-                        GFile *dest)
+static GFile*
+my_handler_confirm_dest (AutoarExtract *arextract,
+                         GFile *dest,
+                         GList *files,
+                         gpointer data)
 {
   char *path, *uri;
+  GList *l;
+
   path = g_file_get_path (dest);
   uri = g_file_get_uri (dest);
   g_print ("Destination Path: %s\n", path);
   g_print ("Destination URI: %s\n", uri);
   g_free (path);
   g_free (uri);
+
+
+  for (l = files; l != NULL; l = l->next) {
+    char *pathname;
+
+    pathname = g_file_get_path (l->data);
+    g_print ("File: %s\n", pathname);
+
+    g_free (pathname);
+  }
+
+  return g_object_ref (dest);
 }
 
 static void
@@ -103,7 +119,7 @@ main (int argc,
   }
 
   g_signal_connect (arextract, "scanned", G_CALLBACK (my_handler_scanned), NULL);
-  g_signal_connect (arextract, "decide-dest", G_CALLBACK (my_handler_decide_dest), NULL);
+  g_signal_connect (arextract, "confirm-dest", G_CALLBACK (my_handler_confirm_dest), NULL);
   g_signal_connect (arextract, "progress", G_CALLBACK (my_handler_progress), NULL);
   g_signal_connect (arextract, "error", G_CALLBACK (my_handler_error), NULL);
   g_signal_connect (arextract, "completed", G_CALLBACK (my_handler_completed), NULL);