Make movie creation use model based validation. Move movie related

logic from movie::create() into Item_Model

3 files changed, 55 insertions, 132 deletions

diff --git a/modules/gallery/controllers/simple_uploader.php b/modules/gallery/controllers/simple_uploader.php
index 7a7e7557..16d1d241 100644
--- a/modules/gallery/controllers/simple_uploader.php
+++ b/modules/gallery/controllers/simple_uploader.php
@@ -79,6 +79,13 @@ class Simple_Uploader_Controller extends Controller {
       } catch (Exception $e) {
         // The Flash uploader has no good way of reporting complex errors, so just keep it simple.
         Kohana_Log::add("error", $e->getMessage() . "\n" . $e->getTraceAsString());
+
+        // Ugh.  I hate to use instanceof, But this beats catching the exception separately since
+        // we mostly want to treat it the same way as all other exceptions
+        if ($e instanceof ORM_Validation_Exception) {
+          Kohana_Log::add("error", "Validation errors: " . print_r($e->validation->errors(), 1));
+        }
+
         if (file_exists($temp_filename)) {
           unlink($temp_filename);
         }
diff --git a/modules/gallery/helpers/movie.php b/modules/gallery/helpers/movie.php
index b0d24f68..0a27ac94 100644
--- a/modules/gallery/helpers/movie.php
+++ b/modules/gallery/helpers/movie.php
@@ -24,109 +24,6 @@
  * Note: by design, this class does not do any permission checking.
  */
 class movie_Core {
-  /**
-   * Create a new movie.
-   * @param integer $parent_id id of parent album
-   * @param string  $filename path to the photo file on disk
-   * @param string  $name the filename to use for this photo in the album
-   * @param integer $title the title of the new photo
-   * @param string  $description (optional) the longer description of this photo
-   * @param string  $slug (optional) the url component for this photo
-   * @return Item_Model
-   */
-  static function create($parent, $filename, $name, $title,
-                         $description=null, $owner_id=null, $slug=null) {
-    if (!$parent->loaded() || !$parent->is_album()) {
-      throw new Exception("@todo INVALID_PARENT");
-    }
-
-    if (!is_file($filename)) {
-      throw new Exception("@todo MISSING_MOVIE_FILE");
-    }
-
-    if (strpos($name, "/")) {
-      throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH");
-    }
-
-    // We don't allow trailing periods as a security measure
-    // ref: http://dev.kohanaphp.com/issues/684
-    if (rtrim($name, ".") != $name) {
-      throw new Exception("@todo NAME_CANNOT_END_IN_PERIOD");
-    }
-
-    try {
-      $movie_info = movie::getmoviesize($filename);
-    } catch (Exception $e) {
-      // Assuming this is MISSING_FFMPEG for now
-      $movie_info = getimagesize(MODPATH . "gallery/images/missing_movie.png");
-    }
-
-    // Force an extension onto the name
-    $pi = pathinfo($filename);
-    if (empty($pi["extension"])) {
-      $pi["extension"] = image_type_to_extension($movie_info[2], false);
-      $name .= "." . $pi["extension"];
-    }
-
-    if (empty($slug)) {
-      $slug = item::convert_filename_to_slug($name);
-    }
-
-    $movie = ORM::factory("item");
-    $movie->type = "movie";
-    $movie->title = $title;
-    $movie->description = $description;
-    $movie->name = $name;
-    $movie->owner_id = $owner_id ? $owner_id : identity::active_user()->id;
-    $movie->width = $movie_info[0];
-    $movie->height = $movie_info[1];
-    $movie->mime_type = strtolower($pi["extension"]) == "mp4" ? "video/mp4" : "video/x-flv";
-    $movie->thumb_dirty = 1;
-    $movie->resize_dirty = 1;
-    $movie->sort_column = "weight";
-    $movie->slug = $slug;
-
-    // Randomize the name if there's a conflict
-    // @todo Improve this.  Random numbers are not user friendly
-    while (ORM::factory("item")
-           ->where("parent_id", "=", $parent->id)
-           ->and_open()
-           ->where("name", "=", $movie->name)
-           ->or_where("slug", "=", $movie->slug)
-           ->close()
-           ->find()->id) {
-      $rand = rand();
-      $movie->name = "{$name}.$rand.{$pi['extension']}";
-      $movie->slug = "{$slug}-$rand";
-    }
-
-    // This saves the photo
-    $movie->add_to_parent($parent);
-
-    // If the thumb or resize already exists then rename it
-    if (file_exists($movie->resize_path()) ||
-        file_exists($movie->thumb_path())) {
-      $movie->name = $pi["filename"] . "-" . rand() . "." . $pi["extension"];
-      $movie->save();
-    }
-
-    copy($filename, $movie->file_path());
-
-    // @todo: publish this from inside Item_Model::save() when we refactor to the point where
-    // there's only one save() happening here.
-    module::event("item_created", $movie);
-
-    // Build our thumbnail
-    graphics::generate($movie);
-
-    // If the parent has no cover item, make this it.
-    if (access::can("edit", $parent) && $parent->album_cover_item_id == null)  {
-      item::make_album_cover($movie);
-    }
-
-    return $movie;
-  }
-
   static function get_edit_form($movie) {
     $form = new Forge("movies/update/$movie->id", "", "post", array("id" => "g-edit-movie-form"));
     $form->hidden("from_id");
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index a9607699..c007afeb 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -376,7 +376,6 @@ class Item_Model extends ORM_MPTT {
     unset($significant_changes["relative_url_cache"]);
     unset($significant_changes["relative_path_cache"]);
 
-
     if (!empty($this->changed) && $significant_changes) {
       $this->updated = time();
       if (!$this->loaded()) {
@@ -403,23 +402,32 @@ class Item_Model extends ORM_MPTT {
           $this->slug = trim($tmp, "-");
         }
 
+        // Get the width, height and mime type from our data file for photos and movies.
         if ($this->is_movie() || $this->is_photo()) {
-          $image_info = getimagesize($this->data_file);
+          $pi = pathinfo($this->data_file);
 
           if ($this->is_photo()) {
+            $image_info = getimagesize($this->data_file);
             $this->width = $image_info[0];
             $this->height = $image_info[1];
-            $this->mime_type =
-              empty($image_info['mime']) ? "application/unknown" : $image_info['mime'];
-          }
+            $this->mime_type = $image_info["mime"];
 
-          // Force an extension onto the name if necessary
-          $pi = pathinfo($this->data_file);
-          if (empty($pi["extension"])) {
-            $pi["extension"] = image_type_to_extension($image_info[2], false);
-            $this->name .= "." . $pi["extension"];
-          }
+            // Force an extension onto the name if necessary
+            if (empty($pi["extension"])) {
+              $pi["extension"] = image_type_to_extension($image_info[2], false);
+              $this->name .= "." . $pi["extension"];
+            }
+          } else {
+            list ($this->width, $this->height) = movie::getmoviesize($this->data_file);
 
+            // No extension?  Assume FLV.
+            if (empty($pi["extension"])) {
+              $pi["extension"] = "flv";
+              $this->name .= "." . $pi["extension"];
+            }
+
+            $this->mime_type = strtolower($pi["extension"]) == "mp4" ? "video/mp4" : "video/x-flv";
+          }
         }
 
         // Randomize the name or slug if there's a conflict.  Preserve the extension.
@@ -445,8 +453,9 @@ class Item_Model extends ORM_MPTT {
 
         parent::save();
 
-        // Build our url caches and save again.  If we could depend on a save happening later we
-        // could defer this 2nd save.
+        // Build our url caches, then save again.  We have to do this after it's already been
+        // saved once because we use only information from the database to build the paths.  If we
+        // could depend on a save happening later we could defer this 2nd save.
         $this->_build_relative_caches();
         parent::save();
 
@@ -459,6 +468,7 @@ class Item_Model extends ORM_MPTT {
           break;
 
         case "photo":
+        case "movie":
           // The thumb or resize may already exist in the case where a movie and a photo generate
           // a thumbnail of the same name (eg, foo.flv movie and foo.jpg photo will generate
           // foo.jpg thumbnail).  If that happens, randomize and save again.
@@ -746,26 +756,27 @@ class Item_Model extends ORM_MPTT {
    * Add some custom per-instance rules.
    */
   public function validate($array=null) {
+    // validate() is recursive, only modify the rules on the outermost call.
     if (!$array) {
-      // The root item has different rules for the name and slug.
       if ($this->id == 1) {
+        // Root album can't have a name or slug
         $this->rules["name"] = array("rules" => array("length[0]"));
         $this->rules["slug"] = array("rules" => array("length[0]"));
+      } else {
+        // Layer some callbacks on top of the existing rules
+        $this->rules["name"]["callbacks"] = array(array($this, "valid_name"));
+        $this->rules["slug"]["callbacks"] = array(array($this, "valid_slug"));
       }
 
-      // Names and slugs can't conflict
-      $this->rules["name"]["callbacks"][] = array($this, "valid_name");
-      $this->rules["slug"]["callbacks"][] = array($this, "valid_slug");
-    }
+      // Movies and photos must have data files
+      if (($this->is_photo() || $this->is_movie()) && !$this->loaded()) {
+        $this->rules["name"]["callbacks"][] = array($this, "valid_data_file");
+      }
 
-    // Movies and photos must have data files
-    if (($this->is_photo() || $this->is_movie()) && !$this->loaded()) {
-      $this->rules["name"]["callbacks"][] = array($this, "valid_data_file");
+      // All items must have a legal parent
+      $this->rules["parent_id"]["callbacks"] = array(array($this, "valid_parent"));
     }
 
-    // All items must have a legal parent
-    $this->rules["parent_id"]["callbacks"][] = array($this, "valid_parent");
-
     parent::validate($array);
   }
 
@@ -801,11 +812,19 @@ class Item_Model extends ORM_MPTT {
     }
 
     if ($this->is_movie() || $this->is_photo()) {
-      $new_ext = pathinfo($this->name, PATHINFO_EXTENSION);
-      $old_ext = pathinfo($this->original()->name, PATHINFO_EXTENSION);
-      if (strcasecmp($new_ext, $old_ext)) {
-        $v->add_error("name", "illegal_extension");
-        return;
+      if ($this->loaded()) {
+        // Existing items can't change their extension
+        $new_ext = pathinfo($this->name, PATHINFO_EXTENSION);
+        $old_ext = pathinfo($this->original()->name, PATHINFO_EXTENSION);
+        if (strcasecmp($new_ext, $old_ext)) {
+          $v->add_error("name", "illegal_extension");
+          return;
+        }
+      } else {
+        // New items must have an extension
+        if (!pathinfo($this->name, PATHINFO_EXTENSION)) {
+          $v->add_error("name", "illegal_extension");
+        }
       }
     }