summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBharat Mediratta <bharat@menalto.com>2010-01-14 21:04:09 -0800
committerBharat Mediratta <bharat@menalto.com>2010-01-14 21:04:09 -0800
commitb3e328c9ff4c3e19df4b6d18da947b759fe0c201 (patch)
treee123d20701bd00442ac695befe194fc05c0043d0
parent8fa9ba636b2e5d8a68c24359e9f6147e86798248 (diff)
Begin the process of converting to model based validation. Right
now only Albums_Controller::update() supports the pattern. All form and controller based validation happening when editing an album has been moved over. Model based validation means that our REST controllers share the same validation as web controllers. We'll have consistency enforced at the model level, which is a Good Thing. The basic pattern is now: 1) Rules are in the model 2) ORM::validate() (which is called by ORM::save() but you can call it directly, too) checks the model for all the rules and throws an ORM_Validation_Exception if there are failures 3) Actions are no longer taken when you call Item_Model::__set(). Instead, they're all queued up and executed when you call Item_Model::save(). Notes: - item::validate_xxx() functions are now in Item_Model:: - We still call $form->validate() because the form can have rules (and forms triggered by events will likely continue to have rules.
-rw-r--r--modules/gallery/controllers/albums.php51
-rw-r--r--modules/gallery/helpers/album.php9
-rw-r--r--modules/gallery/helpers/item.php18
-rw-r--r--modules/gallery/models/item.php139
4 files changed, 106 insertions, 111 deletions
diff --git a/modules/gallery/controllers/albums.php b/modules/gallery/controllers/albums.php
index 2eeefdf1..8ad3ff72 100644
--- a/modules/gallery/controllers/albums.php
+++ b/modules/gallery/controllers/albums.php
@@ -129,42 +129,27 @@ class Albums_Controller extends Items_Controller {
access::required("edit", $album);
$form = album::get_edit_form($album);
- if ($valid = $form->validate()) {
- if ($album->id != 1 &&
- $form->edit_item->dirname->value != $album->name ||
- $form->edit_item->slug->value != $album->slug) {
- // Make sure that there's not a conflict
- if ($row = db::build()
- ->select(array("name", "slug"))
- ->from("items")
- ->where("parent_id", "=", $album->parent_id)
- ->where("id", "<>", $album->id)
- ->and_open()
- ->where("name", "=", $form->edit_item->dirname->value)
- ->or_where("slug", "=", $form->edit_item->slug->value)
- ->close()
- ->execute()
- ->current()) {
- if ($row->name == $form->edit_item->dirname->value) {
- $form->edit_item->dirname->add_error("name_conflict", 1);
- }
- if ($row->slug == $form->edit_item->slug->value) {
- $form->edit_item->slug->add_error("slug_conflict", 1);
- }
- $valid = false;
- }
- }
- }
-
- if ($valid) {
+ try {
+ $valid = $form->validate();
$album->title = $form->edit_item->title->value;
$album->description = $form->edit_item->description->value;
$album->sort_column = $form->edit_item->sort_order->column->value;
$album->sort_order = $form->edit_item->sort_order->direction->value;
- if ($album->id != 1) {
- $album->rename($form->edit_item->dirname->value);
- }
+ $album->name = $form->edit_item->dirname->value;
$album->slug = $form->edit_item->slug->value;
+ $album->validate();
+ } catch (ORM_Validation_Exception $e) {
+ // Translate ORM validation errors into form error messages
+ foreach ($e->validation->errors() as $key => $error) {
+ if ($key == "name") {
+ $key = "dirname";
+ }
+ $form->edit_item->inputs[$key]->add_error($error, 1);
+ }
+ $valid = false;
+ }
+
+ if ($valid) {
$album->save();
module::event("item_edit_form_completed", $album, $form);
@@ -180,9 +165,7 @@ class Albums_Controller extends Items_Controller {
print json_encode(array("result" => "success"));
}
} else {
- print json_encode(
- array("result" => "error",
- "form" => $form->__toString()));
+ print json_encode(array("result" => "error", "form" => (string) $form));
}
}
diff --git a/modules/gallery/helpers/album.php b/modules/gallery/helpers/album.php
index feaf74cc..477f1945 100644
--- a/modules/gallery/helpers/album.php
+++ b/modules/gallery/helpers/album.php
@@ -107,7 +107,6 @@ class album_Core {
t("The internet address should contain only letters, numbers, hyphens and underscores"));
$group->hidden("type")->value("album");
$group->submit("")->value(t("Create"));
- $form->add_rules_from(ORM::factory("item"));
$form->script("")
->url(url::abs_file("modules/gallery/js/albums_form_add.js"));
return $form;
@@ -124,15 +123,12 @@ class album_Core {
$group->input("dirname")->label(t("Directory Name"))->value($parent->name)
->rules("required")
->error_messages(
- "name_conflict", t("There is already a movie, photo or album with this name"))
- ->callback("item::validate_no_slashes")
+ "conflict", t("There is already a movie, photo or album with this name"))
->error_messages("no_slashes", t("The directory name can't contain a \"/\""))
- ->callback("item::validate_no_trailing_period")
->error_messages("no_trailing_period", t("The directory name can't end in \".\""));
$group->input("slug")->label(t("Internet Address"))->value($parent->slug)
->error_messages(
- "slug_conflict", t("There is already a movie, photo or album with this internet address"))
- ->callback("item::validate_url_safe")
+ "conflict", t("There is already a movie, photo or album with this internet address"))
->error_messages(
"not_url_safe",
t("The internet address should contain only letters, numbers, hyphens and underscores"));
@@ -159,7 +155,6 @@ class album_Core {
$group = $form->group("buttons")->label("");
$group->hidden("type")->value("album");
$group->submit("")->value(t("Modify"));
- $form->add_rules_from(ORM::factory("item"));
return $form;
}
diff --git a/modules/gallery/helpers/item.php b/modules/gallery/helpers/item.php
index 1fd9ef16..53291ccc 100644
--- a/modules/gallery/helpers/item.php
+++ b/modules/gallery/helpers/item.php
@@ -78,24 +78,6 @@ class item_Core {
graphics::generate($album);
}
- static function validate_no_slashes($input) {
- if (strpos($input->value, "/") !== false) {
- $input->add_error("no_slashes", 1);
- }
- }
-
- static function validate_no_trailing_period($input) {
- if (rtrim($input->value, ".") !== $input->value) {
- $input->add_error("no_trailing_period", 1);
- }
- }
-
- static function validate_url_safe($input) {
- if (preg_match("/[^A-Za-z0-9-_]/", $input->value)) {
- $input->add_error("not_url_safe", 1);
- }
- }
-
/**
* Sanitize a filename into something presentable as an item title
* @param string $filename
diff --git a/modules/gallery/models/item.php b/modules/gallery/models/item.php
index 4a3d26e9..19bdf655 100644
--- a/modules/gallery/models/item.php
+++ b/modules/gallery/models/item.php
@@ -21,11 +21,11 @@ class Item_Model extends ORM_MPTT {
protected $children = 'items';
protected $sorting = array();
- var $form_rules = array(
- "name" => "required|length[0,255]",
- "title" => "required|length[0,255]",
- "description" => "length[0,65535]",
- "slug" => "required|length[0,255]"
+ var $rules = array(
+ "name" => array("rules" => array("length[0,255]", "required")),
+ "title" => array("rules" => array("length[0,255]", "required")),
+ "slug" => array("rules" => array("length[0,255]", "required")),
+ "description" => array("rules" => array("length[0,65535]"))
);
/**
@@ -146,21 +146,12 @@ class Item_Model extends ORM_MPTT {
}
/**
- * Rename the underlying file for this item to a new name. Move all the files. This requires a
- * save.
+ * Rename the underlying file for this item to a new name and move all related files.
*
* @chainable
*/
- public function rename($new_name) {
- if ($new_name == $this->name) {
- return;
- }
-
- if (strpos($new_name, "/")) {
- throw new Exception("@todo NAME_CANNOT_CONTAIN_SLASH");
- }
-
- $old_relative_path = urldecode($this->relative_path());
+ private function rename($new_name) {
+ $old_relative_path = urldecode($this->original()->relative_path());
$new_relative_path = dirname($old_relative_path) . "/" . $new_name;
if (file_exists(VARPATH . "albums/$new_relative_path")) {
throw new Exception("@todo INVALID_RENAME_FILE_EXISTS: $new_relative_path");
@@ -178,18 +169,6 @@ class Item_Model extends ORM_MPTT {
@rename(VARPATH . "thumbs/$old_relative_path", VARPATH . "thumbs/$new_relative_path");
}
- $this->name = $new_name;
-
- if ($this->is_album()) {
- db::build()
- ->update("items")
- ->set("relative_url_cache", null)
- ->set("relative_path_cache", null)
- ->where("left_ptr", ">", $this->left_ptr)
- ->where("right_ptr", "<", $this->right_ptr)
- ->execute();
- }
-
return $this;
}
@@ -376,29 +355,6 @@ class Item_Model extends ORM_MPTT {
}
/**
- * @see ORM::__set()
- */
- public function __set($column, $value) {
- if ($column == "name") {
- $this->relative_path_cache = null;
- } else if ($column == "slug") {
- if ($this->slug != $value) {
- // Clear the relative url cache for this item and all children
- $this->relative_url_cache = null;
- if ($this->is_album()) {
- db::build()
- ->update("items")
- ->set("relative_url_cache", null)
- ->where("left_ptr", ">", $this->left_ptr)
- ->where("right_ptr", "<", $this->right_ptr)
- ->execute();
- }
- }
- }
- parent::__set($column, $value);
- }
-
- /**
* @see ORM::save()
*/
public function save() {
@@ -414,9 +370,34 @@ class Item_Model extends ORM_MPTT {
$this->weight = item::get_max_weight();
} else {
$send_event = 1;
+
+ if ($this->original()->name != $this->name) {
+ $this->rename($this->name);
+ $this->relative_path_cache = null;
+ }
+
+ if ($this->original()->slug != $this->slug) {
+ // Clear the relative url cache for this item and all children
+ $this->relative_url_cache = null;
+ }
+
+ // Changing the name or the slug ripples downwards
+ if ($this->is_album() &&
+ ($this->original()->name != $this->name ||
+ $this->original()->slug != $this->slug)) {
+ db::build()
+ ->update("items")
+ ->set("relative_url_cache", null)
+ ->set("relative_path_cache", null)
+ ->where("left_ptr", ">", $this->left_ptr)
+ ->where("right_ptr", "<", $this->right_ptr)
+ ->execute();
+ }
}
}
+
parent::save();
+
if (isset($send_event)) {
module::event("item_updated", $this->original(), $this);
}
@@ -655,4 +636,58 @@ class Item_Model extends ORM_MPTT {
}
return parent::descendants($limit, $offset, $where, $order_by);
}
+
+ /**
+ * Add some custom per-instance rules.
+ */
+ public function validate($array=null) {
+ if (!$array) {
+ // The root item has different rules for the name and slug.
+ if ($this->id == 1) {
+ $this->rules["name"]["rules"][] = "length[0]";
+ $this->rules["slug"]["rules"][] = "length[0]";
+ }
+
+ // Names and slugs can't conflict
+ $this->rules["name"]["callbacks"][] = array($this, "valid_name");
+ $this->rules["slug"]["callbacks"][] = array($this, "valid_slug");
+ }
+
+ parent::validate($array);
+ }
+
+ /**
+ * Validate that the desired slug does not conflict.
+ */
+ public function valid_slug(Validation $v, $value) {
+ if (preg_match("/[^A-Za-z0-9-_]/", $value)) {
+ $v->add_error("slug", "not_url_safe");
+ } else if ($row = db::build()
+ ->from("items")
+ ->where("parent_id", "=", $this->parent_id)
+ ->where("id", "<>", $this->id)
+ ->where("slug", "=", $this->slug)
+ ->count_records()) {
+ $v->add_error("slug", "conflict");
+ }
+ }
+
+ /**
+ * Validate the item name. It can't conflict with other names, can't contain slashes or
+ * trailing periods.
+ */
+ public function valid_name(Validation $v, $value) {
+ if (strpos($value, "/") !== false) {
+ $v->add_error("name", "no_slashes");
+ } else if (rtrim($value, ".") !== $value) {
+ $v->add_error("name", "no_trailing_period");
+ } else if ($row = db::build()
+ ->from("items")
+ ->where("parent_id", "=", $this->parent_id)
+ ->where("id", "<>", $this->id)
+ ->where("name", "=", $this->name)
+ ->count_records()) {
+ $v->add_error("name", "conflict");
+ }
+ }
}