2 files changed, 35 insertions, 88 deletions

diff --git a/modules/tag/helpers/tag.php b/modules/tag/helpers/tag.php
index ba8a438e..1fb2e940 100644
--- a/modules/tag/helpers/tag.php
+++ b/modules/tag/helpers/tag.php
@@ -51,59 +51,6 @@ class tag_Core {
   }
 
   /**
-   * Modify the tags associate with an item.
-   *
-   * @param Item_Model $item an item
-   * @param string     $new_tags_string a string of new tags name seperated by ;
-   * @return null
-   * @throws Exception("@todo {$tag_name} WAS_NOT_ADDED_TO {$item->id}")
-   * @throws Exception("@todo {$tag_name} WAS_NOT_DELETED_TO {$item->id}")
-   */
-  static function update($item, $new_tags_string) {
-    $old_tags = self::get_tags($item);
-
-    $new_tags = preg_split("/[,;]/", $new_tags_string);
-    foreach ($new_tags as $i => $new_tag) {
-      $new_tags[$i] = trim($new_tag);
-    }
-
-    $add_tags = array_diff($new_tags, $old_tags);
-    foreach ($add_tags as $tag_name) {
-      if (empty($tag_name)) continue;
-      $tag = ORM::factory("tag")->where("name", $tag_name)->find();
-      if (!$tag->loaded) {
-        $tag->name = $tag_name;
-        $tag->count = 0;
-        $tag->save();
-      }
-      if (!$tag->has($item)) {
-        if (!$tag->add($item, $tag)) {
-          throw new Exception("@todo {$tag->name} WAS_NOT_ADDED_TO {$item->id}");
-        }
-        $tag->count++;
-        $tag->save();
-      }
-    }
-    
-    $del_tags = array_diff($old_tags, $new_tags);
-    foreach ($del_tags as $tag_name) {
-      $tag = ORM::factory("tag")->where("name", $tag_name)->find();
-      if ($tag->has($item)) {
-        if (!$tag->remove($item, $tag)) {
-          throw new Exception("@todo {$tag->name} WAS_NOT_DELETED_TO {$item->id}");
-        }
-        $tag->save();
-        $tag->count--;
-        if ($tag->count <= 0) {
-          $tag->delete();
-        } else {
-          $tag->save();
-        }
-      }
-    }
-  }
-
-  /**
    * Return the N most popular tags.
    *
    * @return ORM_Iterator of Tag_Model in descending tag count order
@@ -180,18 +127,24 @@ class tag_Core {
     $group->submit("")->value(t("Delete Tag"));
     return $form;
   }
-  
-  static function get_tags($item) {
-    $records = ORM::factory("item")
-                    ->select("tags.name as tag_name")
-                    ->join("items_tags", "items.id", "items_tags.item_id", "left")
-                    ->join("tags", "items_tags.tag_id", "tags.id", "left")
-                    ->where("items.id", $item->id)
-                    ->find_all();
-    $tags = array(); 
-    foreach ($records as $record) {
-      $tags[] = $record->tag_name;
-    }
-    return $tags; 
+
+  /**
+   * Delete all tags associated with an item
+   */
+  static function clear_all($item) {
+    $db = Database::instance();
+    $db->query("UPDATE {tags} SET `count` = `count` - 1 WHERE `count` > 0 " .
+               "AND `id` IN (SELECT `tag_id` from {items_tags} WHERE `item_id` = $item->id)");
+    $db->delete("items_tags", array("item_id" => "$item->id"));
+  }
+
+  /**
+   * Get rid of any tags that have no associated items.
+   */
+  static function compact() {
+    // @todo There's a potential race condition here which we can solve by adding a lock around
+    // this and all the cases where we create/update tags.  I'm loathe to do that since it's an
+    // extremely rare case.
+    Database::instance() ->delete("tags", array("count" => 0));
   }
 }
\ No newline at end of file
diff --git a/modules/tag/helpers/tag_event.php b/modules/tag/helpers/tag_event.php
index 0b9504b3..d13d1340 100644
--- a/modules/tag/helpers/tag_event.php
+++ b/modules/tag/helpers/tag_event.php
@@ -60,29 +60,23 @@ class tag_event_Core {
   }
 
   static function item_deleted($item) {
-    $db = Database::instance();
-    $db->query("UPDATE {tags} SET `count` = `count` - 1 WHERE `count` > 0 " .
-               "AND `id` IN (SELECT `tag_id` from {items_tags} WHERE `item_id` = $item->id)");
-    $db->query("DELETE FROM {tags} WHERE `count` = 0 AND `id` IN (" .
-               "SELECT `tag_id` from {items_tags} WHERE `item_id` = $item->id)");
-    $db->delete("items_tags", array("item_id" => "$item->id"));
+    tag::clear_all($item);
+    tag::compact();
   }
-  
-  static function album_edit_form($album, $form) {
-    $tag_value = implode('; ', tag::get_tags($album));
-    $form->edit_album->input("tags")->label(t("Tags ( seperate by , or ; )"))->value($tag_value);
-  }
-  
-  static function album_edit_form_completed($album, $form) {
-    tag::update($album, $form->edit_album->tags->value);
-  }
-  
-  static function photo_edit_form($photo, $form) {
-    $tag_value = implode('; ', tag::get_tags($photo));
-    $form->edit_photo->input("tags")->label(t("Tags ( seperate by , or ; )"))->value($tag_value);
+
+  static function item_edit_form($item, $form) {
+    $tag_value = implode("; ", tag::item_tags($item));
+    $form->edit_item->input("tags")->label(t("Tags (separate by , or ;)"))
+      ->value($tag_value);
   }
-  
-  static function photo_edit_form_completed($photo, $form) {
-    tag::update($photo, $form->edit_photo->tags->value);
+
+  static function item_edit_form_completed($item, $form) {
+    tag::clear_all($item);
+    foreach (preg_split("/[,;]/", $form->edit_item->tags->value) as $tag_name) {
+      if ($tag_name) {
+        tag::add($item, $tag_name);
+      }
+    }
+    tag::compact();
   }
 }