Fix for ticket #1118. Create a item::save_with_retries helper method, which encapsulates saving an item and handling name and slug conflicts. Call this instead of doing a save directly.

3 files changed, 21 insertions, 14 deletions

diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php
index c7e5031b..8ac1fc8b 100644
--- a/modules/gallery/controllers/simple_uploader.php
+++ b/modules/gallery/controllers/simple_uploader.php
@@ -65,12 +65,16 @@ class Simple_Uploader_Controller extends Controller {
         if (array_key_exists("extension", $path_info) &&
             in_array(strtolower($path_info["extension"]), array("flv", "mp4"))) {
           $item->type = "movie";
-          $item->save();
+        } else {
+          $item->type = "photo";
+        }
+
+        item::save_with_retries($item);
+
+        if ($item->type == "movie") {
           log::success("content", t("Added a movie"),
                        html::anchor("movies/$item->id", t("view movie")));
         } else {
-          $item->type = "photo";
-          $item->save();
           log::success("content", t("Added a photo"),
                        html::anchor("photos/$item->id", t("view photo")));
         }
diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 6a740de4..0710d8b2 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -43,6 +43,17 @@ class item_Core {
 
     // Moving may result in name or slug conflicts.  If that happens, try up to 5 times to pick a
     // random name (or slug) to avoid the conflict.
+    $message = item::save_with_retries($source);
+
+    // If the target has no cover item, make this it.
+    if ($target->album_cover_item_id == null)  {
+      item::make_album_cover($source);
+    }
+
+    return $message;
+  }
+
+  static function save_with_retries($source, $retries=5) {
     $orig_name = $source->name;
     $orig_name_filename = pathinfo($source->name, PATHINFO_FILENAME);
     $orig_name_extension = pathinfo($source->name, PATHINFO_EXTENSION);
@@ -91,12 +102,6 @@ class item_Core {
         }
       }
     }
-
-    // If the target has no cover item, make this it.
-    if ($target->album_cover_item_id == null)  {
-      item::make_album_cover($source);
-    }
-
     return $message;
   }
 
diff --git a/modules/gallery/helpers/item_rest.php b/modules/gallery/helpers/item_rest.php
index 692d0895..74fab2e7 100644
--- a/modules/gallery/helpers/item_rest.php
+++ b/modules/gallery/helpers/item_rest.php
@@ -112,8 +112,6 @@ class item_rest_Core {
           }
           break;
 
-        case "parent":
-          break;
         default:
           if (property_exists($entity, $key)) {
             $item->$key = $entity->$key;
@@ -126,7 +124,7 @@ class item_rest_Core {
         $parent = rest::resolve($entity->parent);
         item::move($item, $parent);
       } else {
-        $item->save();
+        $item::save_with_retries($item);
       }
     }
 
@@ -157,7 +155,7 @@ class item_rest_Core {
       $item->title = isset($entity->title) ? $entity->title : $entity->name;
       $item->description = isset($entity->description) ? $entity->description : null;
       $item->slug = isset($entity->slug) ? $entity->slug : null;
-      $item->save();
+      $item::save_with_retries($item);
       break;
 
     case "photo":
@@ -172,7 +170,7 @@ class item_rest_Core {
       $item->title = isset($entity->title) ? $entity->title : $entity->name;
       $item->description = isset($entity->description) ? $entity->description : null;
       $item->slug = isset($entity->slug) ? $entity->slug : null;
-      $item->save();
+      $item::save_with_retries($item);
       break;
 
     default: