GNOME.org
 

[gnome-autoar] AutoarExtract: refactor destination decision process

commit de85c5c6994d172be578f8de1245ec0ae77c3610
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.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=768645

 gnome-autoar/autoar-extract.c |  275 +++++++++++++++++++++++++++++------------
 tests/test-extract.c          |   24 +++-
 2 files changed, 213 insertions(+), 86 deletions(-)
---
diff --git a/gnome-autoar/autoar-extract.c b/gnome-autoar/autoar-extract.c
index 6328ee0..e54bb79 100644
--- a/gnome-autoar/autoar-extract.c
+++ b/gnome-autoar/autoar-extract.c
@@ -59,18 +59,26 @@
  * @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.
+ * directories from an archive. By default, it will only create one file or
+ * directory in the output directory. This is done to avoid clutter on the
+ * user's output directory. If the archive contains only one file, the file
+ * will be extracted to the output directory. If the archive has more than one
+ * file, the files will be extracted in a directory having the same name as the
+ * archive, except the extension. It is also possible to just extract all files
+ * to the output directory (note that this will not perform 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::decide-destination is emitted. The signal provides the decided
+ * destination and the list of files to be extracted. The signal also allows a
+ * new output destination to be used instead of the one provided by
+ * #AutoarExtract. This is convenient for solving name conflicts and
+ * implementing specific logic based on the contents of the archive.
  *
- * 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
@@ -117,8 +125,6 @@ struct _AutoarExtractPrivate
   guint64 size;
   guint64 completed_size;
 
-  GList *files_list;
-
   guint files;
   guint completed_files;
 
@@ -130,12 +136,15 @@ struct _AutoarExtractPrivate
   gssize        buffer_size;
   GError       *error;
 
+  GList *files_list;
+
   GHashTable *userhash;
   GHashTable *grouphash;
   GArray     *extracted_dir_list;
-  GFile      *top_level_dir;
+  GFile      *destination_dir;
 
   GFile *prefix;
+  GFile *new_prefix;
 
   char *suggested_destname;
 
@@ -152,7 +161,7 @@ struct _GFileAndInfo
 enum
 {
   SCANNED,
-  DECIDE_DEST,
+  DECIDE_DESTINATION,
   PROGRESS,
   CANCELLED,
   COMPLETED,
@@ -383,17 +392,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 will be notified via #AutoarExtract::decide-destination, when it
+ * is possible to set a new destination.
+ *
+ * #AutoarExtract will attempt to create the destination 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,
@@ -444,9 +454,10 @@ 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));
+  g_clear_object (&(priv->new_prefix));
 
   g_list_free_full (priv->files_list, g_object_unref);
   priv->files_list = NULL;
@@ -689,11 +700,16 @@ autoar_extract_signal_scanned (AutoarExtract *arextract)
 }
 
 static inline void
-autoar_extract_signal_decide_dest (AutoarExtract *arextract)
+autoar_extract_signal_decide_destination (AutoarExtract *arextract,
+                                          GFile *destination,
+                                          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[DECIDE_DESTINATION], 0,
+                               destination,
+                               files,
+                               new_destination);
 }
 
 static inline void
@@ -779,24 +795,47 @@ autoar_extract_get_common_prefix (GList *files,
 }
 
 static GFile*
-autoar_extract_do_sanitize_pathname (const char *pathname,
-                                     GFile *top_level_dir) {
+autoar_extract_do_sanitize_pathname (AutoarExtract *arextract,
+                                     const char *pathname)
+{
+  AutoarExtractPrivate *priv = arextract->priv;
   GFile *extracted_filename;
+  gboolean valid_filename;
+  g_autofree char *sanitized_pathname;
 
-  extracted_filename = g_file_get_child (top_level_dir, pathname);
+  extracted_filename = g_file_get_child (priv->destination_dir, pathname);
 
-  if (!g_file_has_prefix (extracted_filename, top_level_dir)) {
-    char *basename;
+  valid_filename = g_file_equal (extracted_filename, priv->destination_dir) ||
+                   g_file_has_prefix (extracted_filename, priv->destination_dir);
+
+  if (!valid_filename) {
+    g_autofree 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 (priv->destination_dir,
+                                           basename);
+  }
+
+  if (priv->prefix != NULL && priv->new_prefix != NULL) {
+    g_autofree char *relative_path;
+    /* Replace the old prefix with the new one */
+    relative_path = g_file_get_relative_path (priv->prefix,
+                                              extracted_filename);
+
+    relative_path = relative_path != NULL ? relative_path : g_strdup ("");
 
-    g_free (basename);
+    g_object_unref (extracted_filename);
+
+    extracted_filename = g_file_get_child (priv->new_prefix, relative_path);
   }
 
+  sanitized_pathname = g_file_get_path (extracted_filename);
+
+  g_debug ("autoar_extract_do_sanitize_pathname: %s", sanitized_pathname);
+
   return extracted_filename;
 }
 
@@ -1212,22 +1251,29 @@ autoar_extract_class_init (AutoarExtractClass *klass)
                   G_TYPE_UINT);
 
 /**
- * AutoarExtract::decide-dest:
+ * AutoarExtract::decide-destination:
  * @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
+ *
+ * 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 location of the extracted directory or
- * file is determined.
+ * This signal is emitted when the path of the destination 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[DECIDE_DESTINATION] =
+    g_signal_new ("decide-destination",
                   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:
@@ -1331,7 +1377,8 @@ autoar_extract_init (AutoarExtract *arextract)
   priv->grouphash = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL);
   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->new_prefix = NULL;
 
   priv->suggested_destname = NULL;
 
@@ -1471,15 +1518,20 @@ autoar_extract_step_scan_toplevel (AutoarExtract *arextract)
 }
 
 static void
-autoar_extract_step_decide_dest (AutoarExtract *arextract) {
-  /* Step 1: Create necessary directories
-   * If the archive contains only one file, we don't create the directory */
+autoar_extract_step_set_destination (AutoarExtract *arextract)
+{
+  /* Step 1: Set destination based on client preferences or archive contents */
 
   AutoarExtractPrivate *priv;
 
   priv = arextract->priv;
 
-  g_debug ("autoar_extract_step_decide_dest: called");
+  g_debug ("autoar_extract_step_set_destination: called");
+
+  if (priv->output_is_dest) {
+    priv->destination_dir = g_object_ref (arextract->priv->output_file);
+    return;
+  }
 
   if (priv->prefix != NULL) {
     /* We must check if the archive and the prefix have the same name (without
@@ -1493,42 +1545,100 @@ 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);
+    } else {
+      g_clear_object (&priv->prefix);
     }
   }
-  /* If none of the above situations apply, the top level directory has the
-   * name of the source archive (without 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_destination (AutoarExtract *arextract)
+{
+  /* Step 2: Decide destination */
 
-  if (g_error_matches (priv->error, G_IO_ERROR, G_IO_ERROR_EXISTS)) {
-    g_clear_error (&priv->error);
+  AutoarExtractPrivate *priv;
+  GList *files = NULL;
+  GList *l;
+  GFile *new_destination = NULL;
+  g_autofree char *destination_name;
+
+  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);
+
+  /* When it exists, the common prefix is the actual output of the extraction
+   * and the client has the opportunity to change it. Also, the old prefix is
+   * needed in order to replace it with the new one
+   */
+  if (priv->prefix != NULL) {
+    autoar_extract_signal_decide_destination (arextract,
+                                              priv->prefix,
+                                              files,
+                                              &new_destination);
+
+    priv->new_prefix = new_destination;
+  } else {
+    autoar_extract_signal_decide_destination (arextract,
+                                              priv->destination_dir,
+                                              files,
+                                              &new_destination);
+
+    if (new_destination) {
+      g_object_unref (priv->destination_dir);
+      priv->destination_dir = new_destination;
+    }
   }
 
-  autoar_extract_signal_decide_dest (arextract);
-}
+  destination_name = g_file_get_path (priv->new_prefix != NULL ?
+                                      priv->new_prefix :
+                                      priv->destination_dir);
+  g_debug ("autoar_extract_step_decide_destination: destination %s", destination_name);
 
-static void
-autoar_extract_step_decide_dest_already (AutoarExtract *arextract) {
-  /* Alternative step 1: Output is destination */
-  arextract->priv->top_level_dir = g_object_ref (arextract->priv->output_file);
-  autoar_extract_signal_decide_dest (arextract);
+  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) {
+      /* FIXME: Implement a way to solve directory conflicts */
+      g_debug ("autoar_extract_step_decide_destination: 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 2: Extract files
-   * We have to re-open the archive to extract files */
+  /* Step 3: Extract files
+   * We have to re-open the archive to extract files
+   */
 
   struct archive *a;
   struct archive_entry *entry;
@@ -1565,11 +1675,11 @@ autoar_extract_step_extract (AutoarExtract *arextract) {
     hardlink_filename = NULL;
 
     extracted_filename =
-      autoar_extract_do_sanitize_pathname (pathname, priv->top_level_dir);
+      autoar_extract_do_sanitize_pathname (arextract, pathname);
 
     if (hardlink != NULL) {
       hardlink_filename =
-        autoar_extract_do_sanitize_pathname (hardlink, priv->top_level_dir);
+        autoar_extract_do_sanitize_pathname (arextract, hardlink);
     }
 
     autoar_extract_do_write_entry (arextract, a, entry,
@@ -1601,9 +1711,10 @@ autoar_extract_step_extract (AutoarExtract *arextract) {
 
 static void
 autoar_extract_step_apply_dir_fileinfo (AutoarExtract *arextract) {
-  /* Step 3: Re-apply file info to all directories
+  /* Step 4: Re-apply file info to all directories
    * It is required because modification times may be updated during the
-   * writing of files in the directory. */
+   * writing of files in the directory.
+   */
 
   AutoarExtractPrivate *priv;
   int i;
@@ -1626,9 +1737,10 @@ autoar_extract_step_apply_dir_fileinfo (AutoarExtract *arextract) {
 
 static void
 autoar_extract_step_cleanup (AutoarExtract *arextract) {
-  /* Step 4: Force progress to be 100% and remove the source archive file
+  /* Step 5: 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. */
+   * Errors are not fatal because we have completed our work.
+   */
 
   AutoarExtractPrivate *priv;
 
@@ -1652,7 +1764,7 @@ autoar_extract_run (AutoarExtract *arextract)
 {
   /* Numbers of steps.
    * The array size must be modified if more steps are added. */
-  void (*steps[6])(AutoarExtract*);
+  void (*steps[7])(AutoarExtract*);
 
   AutoarExtractPrivate *priv;
   int i;
@@ -1670,9 +1782,8 @@ autoar_extract_run (AutoarExtract *arextract)
 
   i = 0;
   steps[i++] = autoar_extract_step_scan_toplevel;
-  steps[i++] = priv->output_is_dest ?
-               autoar_extract_step_decide_dest_already :
-               autoar_extract_step_decide_dest;
+  steps[i++] = autoar_extract_step_set_destination;
+  steps[i++] = autoar_extract_step_decide_destination;
   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 b478fdf..a4e9d76 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_decide_destination (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
@@ -83,7 +99,7 @@ main (int argc,
   arextract = autoar_extract_new (source, output, arpref);
 
   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, "decide-destination", G_CALLBACK (my_handler_decide_destination), 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);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]