From d2dd055731903e901674782c8b433a4e418a1b6d Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Wed, 24 Jan 2024 02:22:03 +0300 Subject: [PATCH 1/9] file: Remove unncessary condition This code was supposed to be removed in e70c6e46b77df64715b63847cd471bac49a5da9e. --- src/nautilus-file.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index a65a435821..2038b7ff53 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -1601,8 +1601,6 @@ nautilus_file_poll_for_media (NautilusFile *file) gboolean nautilus_file_can_rename (NautilusFile *file) { - gboolean can_rename; - g_return_val_if_fail (NAUTILUS_IS_FILE (file), FALSE); /* Nonexistent files can't be renamed. */ @@ -1622,13 +1620,6 @@ nautilus_file_can_rename (NautilusFile *file) return FALSE; } - can_rename = TRUE; - - if (!can_rename) - { - return FALSE; - } - return file->details->can_rename; } -- GitLab From 5fa12ae9a6ad05461bfc68b867ab6c7e3cb8a42e Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Thu, 11 Jan 2024 23:15:12 +0300 Subject: [PATCH 2/9] file: Don't skip the first file from operation progress monitoring --- src/nautilus-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index 2038b7ff53..4002080962 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2128,7 +2128,7 @@ real_batch_rename (GList *files, op->renamed_files = 0; op->skipped_files = 0; - for (l1 = files->next; l1 != NULL; l1 = l1->next) + for (l1 = files; l1 != NULL; l1 = l1->next) { file = NAUTILUS_FILE (l1->data); -- GitLab From cb552854d8905502f480401fefd887b03bf06911 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Wed, 24 Jan 2024 02:25:25 +0300 Subject: [PATCH 3/9] file: Don't call callback on every invalid rename This was an error in [1] in which it's calling the passed callback for every renaming failure, which was meant only for when the entire operation ends. It was copied by mistake from the code for single file rename operation. [1] be12a7510090b2ec38229b6e86bc601800d2056b --- src/nautilus-file.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index 4002080962..9e977ba563 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2146,8 +2146,7 @@ real_batch_rename (GList *files, new_file_name = nautilus_file_can_rename_file (file, new_name->str, - callback, - callback_data); + NULL, NULL); if (new_file_name == NULL) { -- GitLab From d2a9a1763bcf9b1eb95661eb877f18589def2bc3 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Tue, 2 Jan 2024 01:25:40 +0300 Subject: [PATCH 4/9] file: Batch rename rework This moves conflict resolution to nautilus-file. This solution tries to canonicalize all changes into a collection of chains and cycles and then operate on them individually in parallel. Example of a cycle: file1->file2 file2->file1 Examples of multiple chains (each one is a separate chain): file1->file01 file2->file02 file3->file03 Example of a single chain: file1->file2 file2->file3 file3->file4 Fixes https://gitlab.gnome.org/GNOME/nautilus/-/issues/1443 --- src/nautilus-file.c | 315 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 259 insertions(+), 56 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index 9e977ba563..f6bea91440 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2103,24 +2103,155 @@ batch_rename_get_info_callback (GObject *source_object, } } +typedef struct +{ + GPtrArray *chain_files; + gboolean is_cycle; + + NautilusFileOperation *op; + GAsyncQueue *rollback_queue; +} RenameChainData; + +typedef struct +{ + GList *old_files; + GList *new_files; +} TwoLists; + +static void +rename_file_chain (gpointer data, + gpointer user_data) +{ + gboolean skip_files = FALSE; + g_autofree RenameChainData *rename_chain = data; + g_autoptr (GPtrArray) chain_files = rename_chain->chain_files; + gsize len = chain_files->len; + g_autoptr (GFile) snapped_file = NULL; + NautilusFileOperation *op = rename_chain->op; + GList *old_files = NULL, *new_files = NULL; + GAsyncQueue *changed_files = user_data; + + if (rename_chain->is_cycle) + { + /* We have a cycle, break it with a temporary name. */ + g_autofree gchar *random_string = g_uuid_string_random (); + + snapped_file = g_ptr_array_index (chain_files, len - 1); + GFile *renamed_file = g_file_set_display_name (snapped_file, + random_string, + op->cancellable, + NULL); + + if (renamed_file != NULL) + { + chain_files->pdata[len - 1] = renamed_file; + } + else + { + /* Unlikely to be a name conflict, skip the entire chain. */ + g_autoptr (GFile) parent = g_file_get_parent (snapped_file); + GFile *new_file = g_file_get_child (parent, random_string); + TwoLists *rollback_files = g_new0 (TwoLists, 1); + + rollback_files->old_files = g_list_prepend (NULL, g_object_ref (snapped_file)); + rollback_files->new_files = g_list_prepend (NULL, new_file); + g_async_queue_push (rename_chain->rollback_queue, rollback_files); + + skip_files = TRUE; + } + } + + /* Process the chain leading to this file */ + for (uint i = 0; i < len - 1; i++) + { + GFile *src = g_ptr_array_index (chain_files, i + 1); + GFile *target = g_ptr_array_index (chain_files, i); + + if (skip_files) + { + guint remaining = len - 1 - i; + + g_atomic_int_add (&op->skipped_files, remaining); + break; + } + + g_autofree gchar *new_name = g_file_get_basename (target); + g_autoptr (GError) error = NULL; + BatchRenameData *rename_data = g_new0 (BatchRenameData, 1); + + /* Do the renaming. */ + g_autoptr (GFile) renamed_file = g_file_set_display_name (src, + new_name, + op->cancellable, + &error); + + rename_data->op = op; + rename_data->file = nautilus_file_get (src); + + g_file_query_info_async (renamed_file, + NAUTILUS_FILE_DEFAULT_ATTRIBUTES, + G_FILE_QUERY_INFO_NONE, + G_PRIORITY_DEFAULT, + op->cancellable, + batch_rename_get_info_callback, + rename_data); + + if (error != NULL) + { + g_autofree gchar *old_name = g_file_get_basename (src); + g_warning ("Batch rename for file \"%s\" failed: %s", + old_name, error->message); + + /* We need to skip the rest of the files in the chain. */ + skip_files = TRUE; + g_atomic_int_inc (&op->skipped_files); + } + else + { + old_files = g_list_prepend (old_files, g_object_ref (src)); + new_files = g_list_prepend (new_files, g_steal_pointer (&renamed_file)); + } + } + + if (snapped_file != NULL) + { + /* Use the original snapped file to undo the operation correctly. */ + g_set_object (&old_files->data, snapped_file); + } + + TwoLists *changed_files_lists = g_new0 (TwoLists, 1); + + changed_files_lists->old_files = old_files; + changed_files_lists->new_files = new_files; + g_async_queue_push (changed_files, changed_files_lists); +} + static void real_batch_rename (GList *files, GList *new_names, NautilusFileOperationCallback callback, gpointer callback_data) { - GList *l1, *l2, *old_files, *new_files; + GList *l1, *l2; + g_autolist (GFile) old_files = NULL, new_files = NULL; NautilusFileOperation *op; - GFile *location; - GString *new_name; - NautilusFile *file; - GError *error; - GFile *new_file; - BatchRenameData *data; - - error = NULL; - old_files = NULL; - new_files = NULL; + gsize len = 0; + /* `staged_targets` and `staged_files` are for the file => renamed file map + * and a reverse map respectively. */ + g_autoptr (GHashTable) staged_targets = g_hash_table_new (g_file_hash, (GEqualFunc) g_file_equal); + g_autoptr (GHashTable) staged_files = g_hash_table_new_full (g_file_hash, + (GEqualFunc) g_file_equal, + g_object_unref, g_object_unref); + /* `stub_files` are for files that will not or cannot be renamed without + * trying. */ + g_autoptr (GHashTable) stub_files = g_hash_table_new_full (g_file_hash, + (GEqualFunc) g_file_equal, + g_object_unref, NULL); + GHashTableIter hash_iter; + GFile *key, *value; + g_autoptr (GAsyncQueue) changed_files = g_async_queue_new (); + GThreadPool *thread_pool = NULL; + g_autoptr (GAsyncQueue) rollback_files = g_async_queue_new (); /* Set up a batch renaming operation. */ op = nautilus_file_operation_new (files->data, callback, callback_data); @@ -2128,86 +2259,158 @@ real_batch_rename (GList *files, op->renamed_files = 0; op->skipped_files = 0; - for (l1 = files; l1 != NULL; l1 = l1->next) - { - file = NAUTILUS_FILE (l1->data); - - file->details->operations_in_progress = g_list_prepend (file->details->operations_in_progress, - op); - } - + /* Collect valid files in hashtables */ for (l1 = files, l2 = new_names; l1 != NULL && l2 != NULL; l1 = l1->next, l2 = l2->next) { - const char *new_file_name; - file = NAUTILUS_FILE (l1->data); - new_name = l2->data; - - location = nautilus_file_get_location (file); + NautilusFile *file = NAUTILUS_FILE (l1->data); + GString *new_name = l2->data; + GFile *location = nautilus_file_get_location (file); + const char *new_file_name = nautilus_file_can_rename_file (file, new_name->str, + NULL, NULL); - new_file_name = nautilus_file_can_rename_file (file, - new_name->str, - NULL, NULL); + len++; if (new_file_name == NULL) { + g_hash_table_add (stub_files, location); op->skipped_files++; continue; } - g_assert (G_IS_FILE (location)); + g_autoptr (GFile) parent = nautilus_file_get_parent_location (file); + g_autoptr (GFile) target = g_file_get_child (parent, new_file_name); - /* Do the renaming. */ - new_file = g_file_set_display_name (location, - new_file_name, - op->cancellable, - &error); + if (g_hash_table_lookup (stub_files, target) != NULL) + { + /* file depends on a stub, so it becomes a stub itself */ + op->skipped_files++; + g_hash_table_add (stub_files, location); - data = g_new0 (BatchRenameData, 1); - data->op = op; - data->file = file; + continue; + } - g_file_query_info_async (new_file, - NAUTILUS_FILE_DEFAULT_ATTRIBUTES, - 0, - G_PRIORITY_DEFAULT, - op->cancellable, - batch_rename_get_info_callback, - data); + file->details->operations_in_progress = g_list_prepend (file->details->operations_in_progress, + op); + g_assert (g_hash_table_insert (staged_targets, location, target)); + g_assert (g_hash_table_insert (staged_files, g_steal_pointer (&target), location)); + } - if (error != NULL) + thread_pool = g_thread_pool_new ((GFunc) rename_file_chain, changed_files, + MIN (32, len), FALSE, NULL); + + /* Discover and assemble individual dependency chains by walking the hash + * table and push them into thread pools. */ + g_hash_table_iter_init (&hash_iter, staged_targets); + while (g_hash_table_iter_next (&hash_iter, (gpointer *) &key, (gpointer *) &value)) + { + GFile *chain_file = key, *iter_start_file = key; + RenameChainData *rename_chain = g_new0 (RenameChainData, 1); + rename_chain->chain_files = g_ptr_array_new_full (3, g_object_unref); + rename_chain->op = op; + rename_chain->rollback_queue = rollback_files; + + /* Iterate to find out if it's a chain or a cycle. */ + while (TRUE) { - g_warning ("Batch rename for file \"%s\" failed", nautilus_file_get_name (file)); - g_error_free (error); - error = NULL; + GFile *prev_chain_file = g_hash_table_lookup (staged_targets, chain_file); - op->skipped_files++; + if (prev_chain_file == NULL) + { + /* Reached chain start */ + break; + } + + if (g_file_equal (iter_start_file, prev_chain_file)) + { + /* We found a cycle */ + rename_chain->is_cycle = TRUE; + g_ptr_array_add (rename_chain->chain_files, g_object_ref (iter_start_file)); + + break; + } + + chain_file = prev_chain_file; } - else + + GFile *target = chain_file; + + iter_start_file = chain_file; + + /* Walk the reverse hash table to collect a list of the files to be + * renamed in an ordered list, but terminate appropriately with a + * cycle. */ + do { - old_files = g_list_append (old_files, location); - new_files = g_list_append (new_files, new_file); + g_autoptr (GFile) src = NULL; + GFile *new_target = NULL; + + g_ptr_array_add (rename_chain->chain_files, target); + + if (g_hash_table_steal_extended (staged_files, target, (gpointer *) &src, (gpointer *) &new_target)) + { + g_hash_table_remove (staged_targets, new_target); + + if (src == target) + { + src = NULL; + } + + if (rename_chain->is_cycle && g_file_equal (new_target, iter_start_file)) + { + g_object_unref (new_target); + break; + } + } + + target = new_target; } + while (target != NULL); + + g_thread_pool_push (thread_pool, rename_chain, NULL); + g_hash_table_iter_init (&hash_iter, staged_targets); + } + + /* Wait for threads to finish and collect the changes. */ + g_thread_pool_free (thread_pool, FALSE, TRUE); + while (g_async_queue_length (changed_files) > 0) + { + g_autofree TwoLists *changed_files_lists = g_async_queue_pop (changed_files); + + old_files = g_list_concat (changed_files_lists->old_files, old_files); + new_files = g_list_concat (changed_files_lists->new_files, new_files); + } + + /* Attempt to rename back the files that were renamed to temporary names. + * Don't handle any errors though, it's likely futile. */ + while (g_async_queue_length (rollback_files) > 0) + { + g_autofree TwoLists *rollback_file_lists = g_async_queue_pop (changed_files); + g_autoptr (GList) old_file_list = rollback_file_lists->old_files; + g_autoptr (GList) new_file_list = rollback_file_lists->new_files; + + g_file_move (new_file_list->data, old_file_list->data, + G_FILE_COPY_NONE, NULL, NULL, NULL, NULL); } /* Tell the undo manager a batch rename is taking place if at least * a file has been renamed*/ - if (!nautilus_file_undo_manager_is_operating () && op->skipped_files != g_list_length (files)) + if (!nautilus_file_undo_manager_is_operating () && op->skipped_files != len) { op->undo_info = nautilus_file_undo_info_batch_rename_new (g_list_length (new_files)); nautilus_file_undo_info_batch_rename_set_data_pre (NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (op->undo_info), - old_files); + g_steal_pointer (&old_files)); nautilus_file_undo_info_batch_rename_set_data_post (NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (op->undo_info), - new_files); + g_steal_pointer (&new_files)); nautilus_file_undo_manager_set_action (op->undo_info); } - if (op->skipped_files == g_list_length (files)) + if (op->skipped_files == len) { - nautilus_file_operation_complete (op, NULL, error); + nautilus_file_operation_complete (op, NULL, NULL); } } -- GitLab From c3dddce9ee60adf082c05fe9f4714f6bb85449dd Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Sat, 31 Aug 2024 01:38:03 +0300 Subject: [PATCH 5/9] file: Don't update files in info query async callback The function used to update NautilusFile internal information after being renamed is called async, which causes havok since the batch renaming requires strict ordering to not add a NautiluFile to a NautilusDirectory that has the same name. Instead, invalidate the files that changed in idle callbacks. --- src/nautilus-file.c | 132 +++++++++++++++----------------------------- 1 file changed, 45 insertions(+), 87 deletions(-) diff --git a/src/nautilus-file.c b/src/nautilus-file.c index f6bea91440..79f094bb22 100644 --- a/src/nautilus-file.c +++ b/src/nautilus-file.c @@ -2031,78 +2031,6 @@ nautilus_file_rename_handle_file_gone (NautilusFile *file, return FALSE; } -typedef struct -{ - NautilusFileOperation *op; - NautilusFile *file; -} BatchRenameData; - -static void -batch_rename_get_info_callback (GObject *source_object, - GAsyncResult *res, - gpointer callback_data) -{ - NautilusFileOperation *op; - NautilusDirectory *directory; - NautilusFile *existing_file; - char *old_uri; - char *new_uri; - const char *new_name; - GFileInfo *new_info; - GError *error; - BatchRenameData *data; - - data = callback_data; - - op = data->op; - op->file = data->file; - - error = NULL; - new_info = g_file_query_info_finish (G_FILE (source_object), res, &error); - if (new_info != NULL) - { - old_uri = nautilus_file_get_uri (op->file); - - new_name = g_file_info_get_name (new_info); - - directory = op->file->details->directory; - - /* If there was another file by the same name in this - * directory and it is not the same file that we are - * renaming, mark it gone. - */ - existing_file = nautilus_directory_find_file_by_name (directory, new_name); - if (existing_file != NULL && existing_file != op->file) - { - nautilus_file_mark_gone (existing_file); - nautilus_file_changed (existing_file); - } - - update_info_and_name (op->file, new_info); - - new_uri = nautilus_file_get_uri (op->file); - nautilus_directory_moved (old_uri, new_uri); - g_free (new_uri); - g_free (old_uri); - g_object_unref (new_info); - } - - op->renamed_files++; - - if (op->files == NULL || - op->renamed_files + op->skipped_files == g_list_length (op->files)) - { - nautilus_file_operation_complete (op, NULL, error); - } - - g_free (data); - - if (error) - { - g_error_free (error); - } -} - typedef struct { GPtrArray *chain_files; @@ -2177,7 +2105,6 @@ rename_file_chain (gpointer data, g_autofree gchar *new_name = g_file_get_basename (target); g_autoptr (GError) error = NULL; - BatchRenameData *rename_data = g_new0 (BatchRenameData, 1); /* Do the renaming. */ g_autoptr (GFile) renamed_file = g_file_set_display_name (src, @@ -2185,17 +2112,6 @@ rename_file_chain (gpointer data, op->cancellable, &error); - rename_data->op = op; - rename_data->file = nautilus_file_get (src); - - g_file_query_info_async (renamed_file, - NAUTILUS_FILE_DEFAULT_ATTRIBUTES, - G_FILE_QUERY_INFO_NONE, - G_PRIORITY_DEFAULT, - op->cancellable, - batch_rename_get_info_callback, - rename_data); - if (error != NULL) { g_autofree gchar *old_name = g_file_get_basename (src); @@ -2208,6 +2124,7 @@ rename_file_chain (gpointer data, } else { + g_atomic_int_inc (&op->renamed_files); old_files = g_list_prepend (old_files, g_object_ref (src)); new_files = g_list_prepend (new_files, g_steal_pointer (&renamed_file)); } @@ -2252,6 +2169,7 @@ real_batch_rename (GList *files, g_autoptr (GAsyncQueue) changed_files = g_async_queue_new (); GThreadPool *thread_pool = NULL; g_autoptr (GAsyncQueue) rollback_files = g_async_queue_new (); + g_autoptr (GError) skip_error = NULL; /* Set up a batch renaming operation. */ op = nautilus_file_operation_new (files->data, callback, callback_data); @@ -2273,7 +2191,16 @@ real_batch_rename (GList *files, if (new_file_name == NULL) { g_hash_table_add (stub_files, location); - op->skipped_files++; + + if (name_is (file, new_name->str)) + { + /* We are ignoring this rename, and not even adding to the undo list. */ + op->renamed_files++; + } + else + { + op->skipped_files++; + } continue; } @@ -2391,6 +2318,34 @@ real_batch_rename (GList *files, g_file_move (new_file_list->data, old_file_list->data, G_FILE_COPY_NONE, NULL, NULL, NULL, NULL); + + NautilusFile *old_file = nautilus_file_get_existing (old_file_list->data); + NautilusFile *new_file = nautilus_file_get_existing (new_file_list->data); + + if (old_file != NULL) + { + nautilus_file_invalidate_attributes (old_file, NAUTILUS_FILE_ATTRIBUTE_INFO); + } + if (new_file != NULL) + { + nautilus_file_invalidate_attributes (new_file, NAUTILUS_FILE_ATTRIBUTE_INFO); + } + } + + /* Invalidate attributes of modified files in idle. */ + for (l1 = old_files, l2 = new_files; l1 != NULL; l1 = l1->next, l2 = l2->next) + { + NautilusFile *old_file = nautilus_file_get_existing (l1->data); + NautilusFile *new_file = nautilus_file_get_existing (l2->data); + + if (old_file != NULL) + { + nautilus_file_invalidate_attributes (old_file, NAUTILUS_FILE_ATTRIBUTE_INFO); + } + if (new_file != NULL) + { + nautilus_file_invalidate_attributes (new_file, NAUTILUS_FILE_ATTRIBUTE_INFO); + } } /* Tell the undo manager a batch rename is taking place if at least @@ -2408,10 +2363,13 @@ real_batch_rename (GList *files, nautilus_file_undo_manager_set_action (op->undo_info); } - if (op->skipped_files == len) + if (op->skipped_files > 1) { - nautilus_file_operation_complete (op, NULL, NULL); + skip_error = g_error_new (G_IO_ERROR, G_IO_ERROR_FAILED, + "Skipped %d file(s)", op->skipped_files); } + + nautilus_file_operation_complete (op, NULL, skip_error); } void -- GitLab From 5c87e6921c85f39a086dbbdaf62ab8065aa8db0b Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Mon, 29 Jan 2024 21:10:25 +0300 Subject: [PATCH 6/9] file-undo-operation: Fix batch rename list leak --- src/nautilus-file-undo-operations.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/nautilus-file-undo-operations.c b/src/nautilus-file-undo-operations.c index 759ec8ffcd..8805489aa7 100644 --- a/src/nautilus-file-undo-operations.c +++ b/src/nautilus-file-undo-operations.c @@ -1201,12 +1201,11 @@ batch_rename_redo_func (NautilusFileUndoInfo *info, { NautilusFileUndoInfoBatchRename *self = NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (info); - GList *l, *files; + GList *l; + g_autolist (NautilusFile) files = NULL; NautilusFile *file; GFile *old_file; - files = NULL; - for (l = self->old_files; l != NULL; l = l->next) { old_file = l->data; @@ -1234,12 +1233,11 @@ batch_rename_undo_func (NautilusFileUndoInfo *info, { NautilusFileUndoInfoBatchRename *self = NAUTILUS_FILE_UNDO_INFO_BATCH_RENAME (info); - GList *l, *files; + GList *l; + g_autolist (NautilusFile) files = NULL; NautilusFile *file; GFile *new_file; - files = NULL; - for (l = self->new_files; l != NULL; l = l->next) { new_file = l->data; -- GitLab From b577b39b8745affe267b00f4c4f729e8a42aaf51 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Thu, 11 Jan 2024 16:41:13 +0300 Subject: [PATCH 7/9] test-utilities: Write the file name when creating a hierarchy This will be used to confirm the content after renaming operations. --- test/automated/test-utilities.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/automated/test-utilities.c b/test/automated/test-utilities.c index 90cecd8d9e..8e78cf3ed6 100644 --- a/test/automated/test-utilities.c +++ b/test/automated/test-utilities.c @@ -140,6 +140,10 @@ file_hierarchy_create (const GStrv hier, { g_autoptr (GFileOutputStream) stream = g_file_create (file, G_FILE_CREATE_NONE, NULL, NULL); + g_autofree gchar *name = g_file_get_basename (file); + + g_output_stream_write_all (G_OUTPUT_STREAM (stream), name, strlen (name) + 1, + NULL, NULL, NULL); } g_assert_true (g_file_query_exists (file, NULL)); -- GitLab From 8138f5f020d79ea3aadbbc0925b5dc49d4976c30 Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Thu, 11 Jan 2024 16:35:28 +0300 Subject: [PATCH 8/9] test/file: Add batch rename tests --- test/automated/displayless/test-file.c | 205 +++++++++++++++++++++++++ 1 file changed, 205 insertions(+) diff --git a/test/automated/displayless/test-file.c b/test/automated/displayless/test-file.c index 1dfba7f779..9cfa37c254 100644 --- a/test/automated/displayless/test-file.c +++ b/test/automated/displayless/test-file.c @@ -4,6 +4,8 @@ #include #include #include +#include +#include static void @@ -87,10 +89,205 @@ test_file_sort_with_self (void) g_assert_cmpint (order, ==, 0); } +typedef struct +{ + const gsize len; + GStrv expected_names; + GStrv expected_content; + gboolean *success; +} NautilusFileBatchRenameTestData; + +static void +batch_rename_callback (NautilusFile *file, + GFile *result_location, + GError *error, + gpointer callback_data) +{ + NautilusFileBatchRenameTestData *data = callback_data; + g_autoptr (GStrvBuilder) name_builder = g_strv_builder_new (); + g_autoptr (GStrvBuilder) content_builder = g_strv_builder_new (); + g_auto (GStrv) result_names = NULL, result_content = NULL; + + g_assert_no_error (error); + g_assert_cmpint (data->len, ==, g_strv_length (data->expected_names)); + g_assert_cmpint (data->len, ==, g_strv_length (data->expected_content)); + + for (guint i = 0; i < data->len; i++) + { + g_autoptr (GFile) location = g_file_new_build_filename (test_get_tmp_dir (), + data->expected_names[i], + NULL); + g_autoptr (GFileInfo) info = g_file_query_info (location, NAUTILUS_FILE_DEFAULT_ATTRIBUTES, + G_FILE_QUERY_INFO_NONE, NULL, NULL); + g_autoptr (GFileInputStream) stream = g_file_read (location, NULL, NULL); + gchar content[1024]; + + g_assert_nonnull (stream); + g_assert_true (g_input_stream_read_all (G_INPUT_STREAM (stream), content, sizeof (content), + NULL, NULL, NULL)); + g_assert_true (g_input_stream_close (G_INPUT_STREAM (stream), NULL, NULL)); + + g_strv_builder_add (name_builder, g_file_info_get_display_name (info)); + g_strv_builder_add (content_builder, content); + } + + result_names = g_strv_builder_end (name_builder); + result_content = g_strv_builder_end (content_builder); + + for (guint i = 0; i < data->len; i++) + { + g_autofree gchar *expected_name = g_path_get_basename (data->expected_names[i]); + g_autofree gchar *expected_content = g_path_get_basename (data->expected_content[i]); + + g_assert_cmpstr (result_names[i], ==, expected_name); + g_assert_cmpstr (result_content[i], ==, expected_content); + } + + *data->success = TRUE; +} + +static void +batch_rename_test (const GStrv hierarchy, + const GStrv original_names, + const GStrv expected_names) +{ + g_autolist (NautilusFile) files = NULL; + g_autolist (GString) new_names = NULL; + const gsize len = g_strv_length (expected_names); + gboolean success = FALSE; + NautilusFileBatchRenameTestData data = { len, expected_names, original_names, &success }; + + file_hierarchy_create (hierarchy, ""); + + for (gint i = len - 1; i >= 0; i--) + { + g_autoptr (GFile) location = g_file_new_build_filename (test_get_tmp_dir (), + original_names[i], + NULL); + NautilusFile *file = nautilus_file_get (location); + GString *new_name = g_string_new_take (g_path_get_basename (expected_names[i])); + + files = g_list_prepend (files, file); + new_names = g_list_prepend (new_names, new_name); + } + + nautilus_file_batch_rename (files, new_names, batch_rename_callback, &data); + + g_assert_true (success); + + /* Test undo by changing the expected names */ + data.expected_names = original_names; + success = FALSE; + + test_operation_undo (); + + batch_rename_callback (NULL, NULL, NULL, &data); + + g_assert_true (success); + + test_clear_tmp_dir (); +} + +static void +test_file_batch_rename_cycles (void) +{ + char *test_cases[][2][10] = + { + /* Small cycle */ + {{"file_1", "file_2", NULL}, + {"file_2", "file_1", NULL}}, + /* Medium cycle */ + {{"file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", "file_9", NULL}, + {"file_9", "file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", NULL}}, + /* Multi-cycle */ + {{"file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", NULL}, + {"file_8", "file_3", "file_4", "file_5", "file_6", "file_7", "file_2", "file_1", NULL}}, + }; + + g_test_bug ("https://gitlab.gnome.org/GNOME/nautilus/-/issues/1443"); + + for (guint i = 0; i < G_N_ELEMENTS (test_cases); i++) + { + batch_rename_test (test_cases[i][0], test_cases[i][0], test_cases[i][1]); + } +} + +static void +test_file_batch_rename_chains (void) +{ + char *test_cases[][2][10] = + { + /* Medium chain */ + {{"file_1", "file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", "file_9", NULL}, + {"file_2", "file_3", "file_4", "file_5", "file_6", "file_7", "file_8", "file_9", "file_10", NULL}}, + }; + + for (guint i = 0; i < G_N_ELEMENTS (test_cases); i++) + { + batch_rename_test (test_cases[i][0], test_cases[i][0], test_cases[i][1]); + } +} + +static void +test_file_batch_rename_replace (void) +{ + char *test_cases[][2][10] = + { + /* File Extension replacement */ + {{ + "file_1.jpg", "file_2.jpeg", "file_3.gif", "file_4.png", "file_5.webm", + "file_6.avif", "file_7.jxl", "file_8.jpeg", "file_9.bmp", NULL + }, + { + "file_1.jpg", "file_2.jpg", "file_3.gif", "file_4.png", "file_5.webm", + "file_6.avif", "file_7.jxl", "file_8.jpg", "file_9.bmp", NULL + }}, + }; + + for (guint i = 0; i < G_N_ELEMENTS (test_cases); i++) + { + batch_rename_test (test_cases[i][0], test_cases[i][0], test_cases[i][1]); + } +} + +static void +test_file_batch_rename_different_folders (void) +{ + char *hierarchy[] = + { + "folder1" G_DIR_SEPARATOR_S, + "folder1" G_DIR_SEPARATOR_S "file_1", + "folder2" G_DIR_SEPARATOR_S, + "folder2" G_DIR_SEPARATOR_S "file_1", + NULL + }; + char *test_cases[][2][3] = + { + /* Rename to the same name but in different folders */ + {{ + "folder1" G_DIR_SEPARATOR_S "file_1", + "folder2" G_DIR_SEPARATOR_S "file_1", + NULL + }, + { + "folder1" G_DIR_SEPARATOR_S "file_2", + "folder2" G_DIR_SEPARATOR_S "file_2", + NULL + }}, + }; + + for (guint i = 0; i < G_N_ELEMENTS (test_cases); i++) + { + batch_rename_test (hierarchy, test_cases[i][0], test_cases[i][1]); + } +} + int main (int argc, char *argv[]) { + g_autoptr (NautilusFileUndoManager) undo_manager = nautilus_file_undo_manager_new (); + g_test_init (&argc, &argv, NULL); g_test_set_nonfatal_assertions (); nautilus_ensure_extension_points (); @@ -107,6 +304,14 @@ main (int argc, test_file_sort_order); g_test_add_func ("/file-sort/with-self", test_file_sort_with_self); + g_test_add_func ("/file-batch-rename/cycles", + test_file_batch_rename_cycles); + g_test_add_func ("/file-batch-rename/chains", + test_file_batch_rename_chains); + g_test_add_func ("/file-batch-rename/replace", + test_file_batch_rename_replace); + g_test_add_func ("/file-batch-rename/different-folders", + test_file_batch_rename_different_folders); return g_test_run (); } -- GitLab From 6e0ce9eda748a3c6b925aebbb07fe9ac722dc99d Mon Sep 17 00:00:00 2001 From: Khalid Abu Shawarib Date: Fri, 12 Jan 2024 00:48:44 +0300 Subject: [PATCH 9/9] batch-rename-utilities: Remove reordering function This function doesn't work when there are cycles. Remove it in favor of the new implementation. --- src/nautilus-batch-rename-dialog.c | 3 - src/nautilus-batch-rename-utilities.c | 118 -------------------------- src/nautilus-batch-rename-utilities.h | 7 -- src/nautilus-file-undo-operations.c | 15 ---- 4 files changed, 143 deletions(-) diff --git a/src/nautilus-batch-rename-dialog.c b/src/nautilus-batch-rename-dialog.c index cda5894765..0a7adf4703 100644 --- a/src/nautilus-batch-rename-dialog.c +++ b/src/nautilus-batch-rename-dialog.c @@ -418,9 +418,6 @@ static void begin_batch_rename (NautilusBatchRenameDialog *dialog, GList *new_names) { - batch_rename_sort_lists_for_rename (&dialog->selection, &new_names, NULL, NULL, NULL, FALSE); - - /* do the actual rename here */ nautilus_file_batch_rename (dialog->selection, new_names, NULL, NULL); gtk_widget_set_cursor (GTK_WIDGET (dialog->window), NULL); diff --git a/src/nautilus-batch-rename-utilities.c b/src/nautilus-batch-rename-utilities.c index 7497c0d47d..6eb4bf9bcc 100644 --- a/src/nautilus-batch-rename-utilities.c +++ b/src/nautilus-batch-rename-utilities.c @@ -121,124 +121,6 @@ batch_rename_get_tag_text_representation (TagConstants tag_constants) return tag_text; } -void -batch_rename_sort_lists_for_rename (GList **selection, - GList **new_names, - GList **old_names, - GList **new_files, - GList **old_files, - gboolean is_undo_redo) -{ - GList *new_names_list; - GList *new_names_list2; - GList *files; - GList *files2; - GList *old_names_list = NULL; - GList *new_files_list = NULL; - GList *old_files_list = NULL; - GList *old_names_list2 = NULL; - GList *new_files_list2 = NULL; - GList *old_files_list2 = NULL; - GString *new_file_name; - GString *new_name; - GString *old_name; - GFile *new_file; - GFile *old_file; - NautilusFile *file; - gboolean order_changed = TRUE; - - /* in the following case: - * file1 -> file2 - * file2 -> file3 - * file2 must be renamed first, so because of that, the list has to be reordered - */ - while (order_changed) - { - order_changed = FALSE; - - if (is_undo_redo) - { - old_names_list = *old_names; - new_files_list = *new_files; - old_files_list = *old_files; - } - - for (new_names_list = *new_names, files = *selection; - new_names_list != NULL && files != NULL; - new_names_list = new_names_list->next, files = files->next) - { - g_autoptr (NautilusFile) parent = NULL; - - new_file_name = new_names_list->data; - parent = nautilus_file_get_parent (NAUTILUS_FILE (files->data)); - - if (is_undo_redo) - { - old_names_list2 = old_names_list; - new_files_list2 = new_files_list; - old_files_list2 = old_files_list; - } - - for (files2 = files, new_names_list2 = new_names_list; - files2 != NULL && new_names_list2 != NULL; - files2 = files2->next, new_names_list2 = new_names_list2->next) - { - const char *file_name; - g_autoptr (NautilusFile) parent2 = NULL; - - file_name = nautilus_file_get_name (NAUTILUS_FILE (files2->data)); - new_name = new_names_list2->data; - - parent2 = nautilus_file_get_parent (NAUTILUS_FILE (files2->data)); - - if (files2 != files && g_strcmp0 (file_name, new_file_name->str) == 0 && - parent == parent2) - { - file = NAUTILUS_FILE (files2->data); - - *selection = g_list_remove_link (*selection, files2); - *new_names = g_list_remove_link (*new_names, new_names_list2); - - *selection = g_list_prepend (*selection, file); - *new_names = g_list_prepend (*new_names, new_name); - - if (is_undo_redo) - { - old_name = old_names_list2->data; - new_file = new_files_list2->data; - old_file = old_files_list2->data; - - *old_names = g_list_remove_link (*old_names, old_names_list2); - *new_files = g_list_remove_link (*new_files, new_files_list2); - *old_files = g_list_remove_link (*old_files, old_files_list2); - - *old_names = g_list_prepend (*old_names, old_name); - *new_files = g_list_prepend (*new_files, new_file); - *old_files = g_list_prepend (*old_files, old_file); - } - - order_changed = TRUE; - break; - } - - if (is_undo_redo) - { - old_names_list2 = old_names_list2->next; - new_files_list2 = new_files_list2->next; - old_files_list2 = old_files_list2->next; - } - } - - if (is_undo_redo) - { - old_names_list = old_names_list->next; - new_files_list = new_files_list->next; - old_files_list = old_files_list->next; - } - } - } -} - GString * markup_hightlight_text (const char *label, const gchar *substring, diff --git a/src/nautilus-batch-rename-utilities.h b/src/nautilus-batch-rename-utilities.h index 6da7142091..743378e8b5 100644 --- a/src/nautilus-batch-rename-utilities.h +++ b/src/nautilus-batch-rename-utilities.h @@ -63,10 +63,3 @@ GString* markup_hightlight_text (const char *label, const gchar *background_color); const gchar* batch_rename_get_tag_text_representation (TagConstants tag_constants); - -void batch_rename_sort_lists_for_rename (GList **selection, - GList **new_names, - GList **old_names, - GList **new_files, - GList **old_files, - gboolean is_undo_redo); diff --git a/src/nautilus-file-undo-operations.c b/src/nautilus-file-undo-operations.c index 8805489aa7..ec5ce35e7c 100644 --- a/src/nautilus-file-undo-operations.c +++ b/src/nautilus-file-undo-operations.c @@ -31,7 +31,6 @@ #include "nautilus-file.h" #include "nautilus-file-undo-manager.h" #include "nautilus-batch-rename-dialog.h" -#include "nautilus-batch-rename-utilities.h" #include "nautilus-scheme.h" #include "nautilus-tag-manager.h" @@ -1216,13 +1215,6 @@ batch_rename_redo_func (NautilusFileUndoInfo *info, files = g_list_reverse (files); - batch_rename_sort_lists_for_rename (&files, - &self->new_display_names, - &self->old_display_names, - &self->new_files, - &self->old_files, - TRUE); - nautilus_file_batch_rename (files, self->new_display_names, file_undo_info_operation_callback, self); } @@ -1248,13 +1240,6 @@ batch_rename_undo_func (NautilusFileUndoInfo *info, files = g_list_reverse (files); - batch_rename_sort_lists_for_rename (&files, - &self->old_display_names, - &self->new_display_names, - &self->old_files, - &self->new_files, - TRUE); - nautilus_file_batch_rename (files, self->old_display_names, file_undo_info_operation_callback, self); } -- GitLab