Assets: Remove appended asset when dropping operation fails

When dropping an asset somewhere, it is appended and then a drop operation is
called to actually add it to the scene based on current context. If this drop
operation fails, the appended data-block is now still in the .blend. The user
may not notice and not expect this.
Instead idea is to rollback any changes done by dropping code if the operation
fails, namely removing the appended data-block again.

Adds a new `cancel()` callback which is called if the drop operator returns
`OPERATOR_CANCELLED` to drop-boxes and a generic function to deal with assets
on drop failure.

Also removes the `free_id_on_error` property of the `NODE_OT_add_group`
operator, which was used as ad-hoc solution to get this same behavior.

15 files changed, 121 insertions, 49 deletions

diff --git a/source/blender/editors/screen/screen_ops.c b/source/blender/editors/screen/screen_ops.c
index 0fc7e713bac..47feb49bd44 100644
--- a/source/blender/editors/screen/screen_ops.c
+++ b/source/blender/editors/screen/screen_ops.c
@@ -5567,8 +5567,8 @@ void ED_keymap_screen(wmKeyConfig *keyconf)
 
   /* dropbox for entire window */
   ListBase *lb = WM_dropboxmap_find("Window", 0, 0);
-  WM_dropbox_add(lb, "WM_OT_drop_blend_file", blend_file_drop_poll, blend_file_drop_copy);
-  WM_dropbox_add(lb, "UI_OT_drop_color", UI_drop_color_poll, UI_drop_color_copy);
+  WM_dropbox_add(lb, "WM_OT_drop_blend_file", blend_file_drop_poll, blend_file_drop_copy, NULL);
+  WM_dropbox_add(lb, "UI_OT_drop_color", UI_drop_color_poll, UI_drop_color_copy, NULL);
 
   keymap_modal_set(keyconf);
 }
diff --git a/source/blender/editors/space_clip/space_clip.c b/source/blender/editors/space_clip/space_clip.c
index 2a7c64a83f7..e0a524a79c1 100644
--- a/source/blender/editors/space_clip/space_clip.c
+++ b/source/blender/editors/space_clip/space_clip.c
@@ -639,7 +639,7 @@ static void clip_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Clip", SPACE_CLIP, 0);
 
-  WM_dropbox_add(lb, "CLIP_OT_open", clip_drop_poll, clip_drop_copy);
+  WM_dropbox_add(lb, "CLIP_OT_open", clip_drop_poll, clip_drop_copy, NULL);
 }
 
 static void clip_refresh(const bContext *C, ScrArea *area)
diff --git a/source/blender/editors/space_console/space_console.c b/source/blender/editors/space_console/space_console.c
index b24579d9871..56a6204b385 100644
--- a/source/blender/editors/space_console/space_console.c
+++ b/source/blender/editors/space_console/space_console.c
@@ -196,8 +196,8 @@ static void console_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Console", SPACE_CONSOLE, RGN_TYPE_WINDOW);
 
-  WM_dropbox_add(lb, "CONSOLE_OT_insert", id_drop_poll, id_drop_copy);
-  WM_dropbox_add(lb, "CONSOLE_OT_insert", path_drop_poll, path_drop_copy);
+  WM_dropbox_add(lb, "CONSOLE_OT_insert", id_drop_poll, id_drop_copy, NULL);
+  WM_dropbox_add(lb, "CONSOLE_OT_insert", path_drop_poll, path_drop_copy, NULL);
 }
 
 /* ************* end drop *********** */
diff --git a/source/blender/editors/space_file/space_file.c b/source/blender/editors/space_file/space_file.c
index 178ca264182..66d24c7ab53 100644
--- a/source/blender/editors/space_file/space_file.c
+++ b/source/blender/editors/space_file/space_file.c
@@ -766,7 +766,7 @@ static void file_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Window", SPACE_EMPTY, RGN_TYPE_WINDOW);
 
-  WM_dropbox_add(lb, "FILE_OT_filepath_drop", filepath_drop_poll, filepath_drop_copy);
+  WM_dropbox_add(lb, "FILE_OT_filepath_drop", filepath_drop_poll, filepath_drop_copy, NULL);
 }
 
 static int file_space_subtype_get(ScrArea *area)
diff --git a/source/blender/editors/space_image/space_image.c b/source/blender/editors/space_image/space_image.c
index 69976bc088c..95ca8aba399 100644
--- a/source/blender/editors/space_image/space_image.c
+++ b/source/blender/editors/space_image/space_image.c
@@ -280,7 +280,7 @@ static void image_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Image", SPACE_IMAGE, 0);
 
-  WM_dropbox_add(lb, "IMAGE_OT_open", image_drop_poll, image_drop_copy);
+  WM_dropbox_add(lb, "IMAGE_OT_open", image_drop_poll, image_drop_copy, NULL);
 }
 
 /**
diff --git a/source/blender/editors/space_node/node_add.c b/source/blender/editors/space_node/node_add.c
index 74fa5cdeb9d..699e5c1d165 100644
--- a/source/blender/editors/space_node/node_add.c
+++ b/source/blender/editors/space_node/node_add.c
@@ -323,9 +323,6 @@ static bNodeTree *node_add_group_get_and_poll_group_node_tree(Main *bmain,
     return NULL;
   }
   if ((node_group->type != ntree->type) || !nodeGroupPoll(ntree, node_group)) {
-    if (RNA_boolean_get(op->ptr, "free_id_on_error")) {
-      BKE_id_delete(bmain, node_group);
-    }
     return NULL;
   }
 
@@ -389,8 +386,6 @@ static int node_add_group_invoke(bContext *C, wmOperator *op, const wmEvent *eve
 
 void NODE_OT_add_group(wmOperatorType *ot)
 {
-  PropertyRNA *prop;
-
   /* identifiers */
   ot->name = "Add Node Group";
   ot->description = "Add an existing node group to the current node editor";
@@ -405,13 +400,6 @@ void NODE_OT_add_group(wmOperatorType *ot)
   ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO | OPTYPE_INTERNAL;
 
   RNA_def_string(ot->srna, "name", "Mask", MAX_ID_NAME - 2, "Name", "Data-block name to assign");
-  prop = RNA_def_boolean(
-      ot->srna,
-      "free_id_on_error",
-      false,
-      "Free Group on Error",
-      "Free the named node group data-block if it could not be added to the tree");
-  RNA_def_property_flag(prop, PROP_HIDDEN);
 }
 
 /* ****************** Add File Node Operator  ******************* */
diff --git a/source/blender/editors/space_node/space_node.c b/source/blender/editors/space_node/space_node.c
index 6e6b0a584cf..289b7d9efa1 100644
--- a/source/blender/editors/space_node/space_node.c
+++ b/source/blender/editors/space_node/space_node.c
@@ -692,10 +692,6 @@ static void node_group_drop_copy(wmDrag *drag, wmDropBox *drop)
   ID *id = WM_drag_get_local_ID_or_import_from_asset(drag, 0);
 
   RNA_string_set(drop->ptr, "name", id->name + 2);
-  if (drag->type == WM_DRAG_ASSET) {
-    /* ID just appended, so free it when dropping fails. */
-    RNA_boolean_set(drop->ptr, "free_id_on_error", true);
-  }
 }
 
 static void node_id_drop_copy(wmDrag *drag, wmDropBox *drop)
@@ -724,9 +720,21 @@ static void node_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Node Editor", SPACE_NODE, RGN_TYPE_WINDOW);
 
-  WM_dropbox_add(lb, "NODE_OT_add_group", node_group_drop_poll, node_group_drop_copy);
-  WM_dropbox_add(lb, "NODE_OT_add_file", node_ima_drop_poll, node_id_path_drop_copy);
-  WM_dropbox_add(lb, "NODE_OT_add_mask", node_mask_drop_poll, node_id_drop_copy);
+  WM_dropbox_add(lb,
+                 "NODE_OT_add_group",
+                 node_group_drop_poll,
+                 node_group_drop_copy,
+                 WM_drag_free_imported_drag_ID);
+  WM_dropbox_add(lb,
+                 "NODE_OT_add_file",
+                 node_ima_drop_poll,
+                 node_id_path_drop_copy,
+                 WM_drag_free_imported_drag_ID);
+  WM_dropbox_add(lb,
+                 "NODE_OT_add_mask",
+                 node_mask_drop_poll,
+                 node_id_drop_copy,
+                 WM_drag_free_imported_drag_ID);
 }
 
 /* ************* end drop *********** */
diff --git a/source/blender/editors/space_outliner/outliner_dragdrop.c b/source/blender/editors/space_outliner/outliner_dragdrop.c
index 70aeef76234..5ae94cf2e64 100644
--- a/source/blender/editors/space_outliner/outliner_dragdrop.c
+++ b/source/blender/editors/space_outliner/outliner_dragdrop.c
@@ -1504,10 +1504,10 @@ void outliner_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Outliner", SPACE_OUTLINER, RGN_TYPE_WINDOW);
 
-  WM_dropbox_add(lb, "OUTLINER_OT_parent_drop", parent_drop_poll, NULL);
-  WM_dropbox_add(lb, "OUTLINER_OT_parent_clear", parent_clear_poll, NULL);
-  WM_dropbox_add(lb, "OUTLINER_OT_scene_drop", scene_drop_poll, NULL);
-  WM_dropbox_add(lb, "OUTLINER_OT_material_drop", material_drop_poll, NULL);
-  WM_dropbox_add(lb, "OUTLINER_OT_datastack_drop", datastack_drop_poll, NULL);
-  WM_dropbox_add(lb, "OUTLINER_OT_collection_drop", collection_drop_poll, NULL);
+  WM_dropbox_add(lb, "OUTLINER_OT_parent_drop", parent_drop_poll, NULL, NULL);
+  WM_dropbox_add(lb, "OUTLINER_OT_parent_clear", parent_clear_poll, NULL, NULL);
+  WM_dropbox_add(lb, "OUTLINER_OT_scene_drop", scene_drop_poll, NULL, NULL);
+  WM_dropbox_add(lb, "OUTLINER_OT_material_drop", material_drop_poll, NULL, NULL);
+  WM_dropbox_add(lb, "OUTLINER_OT_datastack_drop", datastack_drop_poll, NULL, NULL);
+  WM_dropbox_add(lb, "OUTLINER_OT_collection_drop", collection_drop_poll, NULL, NULL);
 }
diff --git a/source/blender/editors/space_sequencer/space_sequencer.c b/source/blender/editors/space_sequencer/space_sequencer.c
index b17482cd3a4..cf26d1e3243 100644
--- a/source/blender/editors/space_sequencer/space_sequencer.c
+++ b/source/blender/editors/space_sequencer/space_sequencer.c
@@ -448,9 +448,9 @@ static void sequencer_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Sequencer", SPACE_SEQ, RGN_TYPE_WINDOW);
 
-  WM_dropbox_add(lb, "SEQUENCER_OT_image_strip_add", image_drop_poll, sequencer_drop_copy);
-  WM_dropbox_add(lb, "SEQUENCER_OT_movie_strip_add", movie_drop_poll, sequencer_drop_copy);
-  WM_dropbox_add(lb, "SEQUENCER_OT_sound_strip_add", sound_drop_poll, sequencer_drop_copy);
+  WM_dropbox_add(lb, "SEQUENCER_OT_image_strip_add", image_drop_poll, sequencer_drop_copy, NULL);
+  WM_dropbox_add(lb, "SEQUENCER_OT_movie_strip_add", movie_drop_poll, sequencer_drop_copy, NULL);
+  WM_dropbox_add(lb, "SEQUENCER_OT_sound_strip_add", sound_drop_poll, sequencer_drop_copy, NULL);
 }
 
 /* ************* end drop *********** */
diff --git a/source/blender/editors/space_text/space_text.c b/source/blender/editors/space_text/space_text.c
index ea55eaea388..98faf89f8ae 100644
--- a/source/blender/editors/space_text/space_text.c
+++ b/source/blender/editors/space_text/space_text.c
@@ -367,8 +367,8 @@ static void text_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("Text", SPACE_TEXT, RGN_TYPE_WINDOW);
 
-  WM_dropbox_add(lb, "TEXT_OT_open", text_drop_poll, text_drop_copy);
-  WM_dropbox_add(lb, "TEXT_OT_insert", text_drop_paste_poll, text_drop_paste);
+  WM_dropbox_add(lb, "TEXT_OT_open", text_drop_poll, text_drop_copy, NULL);
+  WM_dropbox_add(lb, "TEXT_OT_insert", text_drop_paste_poll, text_drop_paste, NULL);
 }
 
 /* ************* end drop *********** */
diff --git a/source/blender/editors/space_view3d/space_view3d.c b/source/blender/editors/space_view3d/space_view3d.c
index 98e1d927daf..3d260a9a05b 100644
--- a/source/blender/editors/space_view3d/space_view3d.c
+++ b/source/blender/editors/space_view3d/space_view3d.c
@@ -689,21 +689,41 @@ static void view3d_dropboxes(void)
 {
   ListBase *lb = WM_dropboxmap_find("View3D", SPACE_VIEW3D, RGN_TYPE_WINDOW);
 
-  WM_dropbox_add(lb, "OBJECT_OT_add_named", view3d_ob_drop_poll, view3d_ob_drop_copy);
-  WM_dropbox_add(lb, "OBJECT_OT_drop_named_material", view3d_mat_drop_poll, view3d_id_drop_copy);
-  WM_dropbox_add(
-      lb, "VIEW3D_OT_background_image_add", view3d_ima_bg_drop_poll, view3d_id_path_drop_copy);
-  WM_dropbox_add(
-      lb, "OBJECT_OT_drop_named_image", view3d_ima_empty_drop_poll, view3d_id_path_drop_copy);
-  WM_dropbox_add(lb, "OBJECT_OT_volume_import", view3d_volume_drop_poll, view3d_id_path_drop_copy);
+  WM_dropbox_add(lb,
+                 "OBJECT_OT_add_named",
+                 view3d_ob_drop_poll,
+                 view3d_ob_drop_copy,
+                 WM_drag_free_imported_drag_ID);
+  WM_dropbox_add(lb,
+                 "OBJECT_OT_drop_named_material",
+                 view3d_mat_drop_poll,
+                 view3d_id_drop_copy,
+                 WM_drag_free_imported_drag_ID);
+  WM_dropbox_add(lb,
+                 "VIEW3D_OT_background_image_add",
+                 view3d_ima_bg_drop_poll,
+                 view3d_id_path_drop_copy,
+                 WM_drag_free_imported_drag_ID);
+  WM_dropbox_add(lb,
+                 "OBJECT_OT_drop_named_image",
+                 view3d_ima_empty_drop_poll,
+                 view3d_id_path_drop_copy,
+                 WM_drag_free_imported_drag_ID);
+  WM_dropbox_add(lb,
+                 "OBJECT_OT_volume_import",
+                 view3d_volume_drop_poll,
+                 view3d_id_path_drop_copy,
+                 WM_drag_free_imported_drag_ID);
   WM_dropbox_add(lb,
                  "OBJECT_OT_collection_instance_add",
                  view3d_collection_drop_poll,
-                 view3d_collection_drop_copy);
+                 view3d_collection_drop_copy,
+                 WM_drag_free_imported_drag_ID);
   WM_dropbox_add(lb,
                  "OBJECT_OT_data_instance_add",
                  view3d_object_data_drop_poll,
-                 view3d_id_drop_copy_with_type);
+                 view3d_id_drop_copy_with_type,
+                 WM_drag_free_imported_drag_ID);
 }
 
 static void view3d_widgets(void)
diff --git a/source/blender/windowmanager/WM_api.h b/source/blender/windowmanager/WM_api.h
index 69177f39c04..d5de45c74fd 100644
--- a/source/blender/windowmanager/WM_api.h
+++ b/source/blender/windowmanager/WM_api.h
@@ -678,7 +678,8 @@ struct wmDropBox *WM_dropbox_add(
     ListBase *lb,
     const char *idname,
     bool (*poll)(struct bContext *, struct wmDrag *, const struct wmEvent *event, const char **),
-    void (*copy)(struct wmDrag *, struct wmDropBox *));
+    void (*copy)(struct wmDrag *, struct wmDropBox *),
+    void (*cancel)(struct Main *, struct wmDrag *, struct wmDropBox *));
 ListBase *WM_dropboxmap_find(const char *idname, int spaceid, int regionid);
 
 /* ID drag and drop */
@@ -690,6 +691,10 @@ bool WM_drag_is_ID_type(const struct wmDrag *drag, int idcode);
 struct wmDragAsset *WM_drag_get_asset_data(const struct wmDrag *drag, int idcode);
 struct ID *WM_drag_get_local_ID_or_import_from_asset(const struct wmDrag *drag, int idcode);
 
+void WM_drag_free_imported_drag_ID(struct Main *bmain,
+                                   struct wmDrag *drag,
+                                   struct wmDropBox *drop);
+
 /* Set OpenGL viewport and scissor */
 void wmViewport(const struct rcti *winrct);
 void wmPartialViewport(rcti *drawrct, const rcti *winrct, const rcti *partialrct);
diff --git a/source/blender/windowmanager/WM_types.h b/source/blender/windowmanager/WM_types.h
index 24aa5bc4869..d40c0715a09 100644
--- a/source/blender/windowmanager/WM_types.h
+++ b/source/blender/windowmanager/WM_types.h
@@ -904,6 +904,12 @@ typedef struct wmDropBox {
   void (*copy)(struct wmDrag *, struct wmDropBox *);
 
   /**
+   * If the operator is cancelled (returns `OPERATOR_CANCELLED`), this can be used for cleanup of
+   * `copy()` resources.
+   */
+  void (*cancel)(struct Main *, struct wmDrag *, struct wmDropBox *);
+
+  /**
    * If poll succeeds, operator is called.
    * Not saved in file, so can be pointer.
    */
diff --git a/source/blender/windowmanager/intern/wm_dragdrop.c b/source/blender/windowmanager/intern/wm_dragdrop.c
index 6fdcbab889c..9684c21605a 100644
--- a/source/blender/windowmanager/intern/wm_dragdrop.c
+++ b/source/blender/windowmanager/intern/wm_dragdrop.c
@@ -39,6 +39,7 @@
 #include "BKE_context.h"
 #include "BKE_global.h"
 #include "BKE_idtype.h"
+#include "BKE_lib_id.h"
 
 #include "GPU_shader.h"
 #include "GPU_state.h"
@@ -95,11 +96,13 @@ ListBase *WM_dropboxmap_find(const char *idname, int spaceid, int regionid)
 wmDropBox *WM_dropbox_add(ListBase *lb,
                           const char *idname,
                           bool (*poll)(bContext *, wmDrag *, const wmEvent *, const char **),
-                          void (*copy)(wmDrag *, wmDropBox *))
+                          void (*copy)(wmDrag *, wmDropBox *),
+                          void (*cancel)(struct Main *, wmDrag *, wmDropBox *))
 {
   wmDropBox *drop = MEM_callocN(sizeof(wmDropBox), "wmDropBox");
   drop->poll = poll;
   drop->copy = copy;
+  drop->cancel = cancel;
   drop->ot = WM_operatortype_find(idname, 0);
   drop->opcontext = WM_OP_INVOKE_DEFAULT;
 
@@ -382,6 +385,9 @@ static ID *wm_drag_asset_id_import(wmDragAsset *asset_drag)
 /**
  * When dragging a local ID, return that. Otherwise, if dragging an asset-handle, link or append
  * that depending on what was chosen by the drag-box (currently append only in fact).
+ *
+ * Use #WM_drag_free_imported_drag_ID() as cancel callback of the drop-box, so that the asset
+ * import is rolled back if the drop operator fails.
  */
 ID *WM_drag_get_local_ID_or_import_from_asset(const wmDrag *drag, int idcode)
 {
@@ -402,6 +408,39 @@ ID *WM_drag_get_local_ID_or_import_from_asset(const wmDrag *drag, int idcode)
   return wm_drag_asset_id_import(asset_drag);
 }
 
+/**
+ * \brief Free asset ID imported for cancelled drop.
+ *
+ * If the asset was imported (linked/appended) using #WM_drag_get_local_ID_or_import_from_asset()`
+ * (typically via a #wmDropBox.copy() callback), we want the ID to be removed again if the drop
+ * operator cancels.
+ * This is for use as #wmDropBox.cancel() callback.
+ */
+void WM_drag_free_imported_drag_ID(struct Main *bmain, wmDrag *drag, wmDropBox *drop)
+{
+  if (drag->type != WM_DRAG_ASSET) {
+    return;
+  }
+
+  wmDragAsset *asset_drag = WM_drag_get_asset_data(drag, 0);
+  if (!asset_drag) {
+    return;
+  }
+
+  /* Get name from property, not asset data - it may have changed after importing to ensure
+   * uniqueness (name is assumed to be set from the imported ID name). */
+  char name[MAX_ID_NAME - 2];
+  RNA_string_get(drop->ptr, "name", name);
+  if (!name[0]) {
+    return;
+  }
+
+  ID *id = BKE_libblock_find_name(bmain, asset_drag->id_type, name);
+  if (id) {
+    BKE_id_delete(bmain, id);
+  }
+}
+
 /* ************** draw ***************** */
 
 static void wm_drop_operator_draw(const char *name, int x, int y)
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index e132caede0d..7b047e68aee 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -2820,8 +2820,14 @@ static int wm_handlers_do_intern(bContext *C, wmEvent *event, ListBase *handlers
                   ListBase single_lb = {drag, drag};
                   event->customdata = &single_lb;
 
-                  wm_operator_call_internal(
+                  int op_retval = wm_operator_call_internal(
                       C, drop->ot, drop->ptr, NULL, drop->opcontext, false, event);
+                  OPERATOR_RETVAL_CHECK(op_retval);
+
+                  if ((op_retval & OPERATOR_CANCELLED) && drop->cancel) {
+                    drop->cancel(CTX_data_main(C), drag, drop);
+                  }
+
                   action |= WM_HANDLER_BREAK;
 
                   /* Free the drags. */