Oct232007

CakePHP tip of the day: pay attention to conventions

CakePHP 1.2 sexyness is mainly a result of its convention over configuration methodology. Theoretical discussions aside, conventions save us (developers) valuable time, and allow us to concentrate on where we should put our effort: business logic. Some conventions may go under the radar until you find what you think is a weird bug. Let’s see an example by building a simple action in a controller:

class PostsController extends AppController {
	function edit($slug) {
		$post = $this->Post->findBySlug($slug);

		if (!empty($this->data)) {
			// We have the post already, so don't let user submit and ID
			// in POSTed information, just in case he gets funky ideas

			if (isset($this->data['Post']['id'])) {
				unset($this->data['Post']['id']);
			}

			$this->Post->set($this->data);
			if ($this->Post->validates()) {
				// Force correct ID
				$this->data['Post']['id'] = $post['Post']['id'];

				// Do something else we need to do before saving
				// ...

				// and now save
				$this->Post->save($this->data);
			}
		}
	}
}

To our surprise, you’ll see that the above code always generates a new Post, instead of editing the post which slug is $slug. Why is that? I’ll give you a hint: print out $this->Post->id just before you execute the validates(), and you’ll see that it is set to whatever the slug is. What? So ID is set to a slug? Now, why the heck is that? Let’s dive into CakePHP core code, and pay special attention to the first few lines of Controller::constructClasses():

if (empty($this->passedArgs) || !isset($this->passedArgs['0'])) {
	$id = false;
} else {
	$id = $this->passedArgs['0'];
}

if ($this->uses === false) {
	$this->loadModel($this->modelClass, $id);
} elseif ($this->uses) {
	$uses = is_array($this->uses) ? $this->uses : array($this->uses);
	$this->modelClass = $uses[0];
	foreach ($uses as $modelClass) {
		$this->loadModel($modelClass);
	}
}

As you can see CakePHP, when only the auto-model linked to our controller is used (i.e: we’re not using Controller::$uses), will assume that the first parameter sent to the current action (if there’s indeed one) corresponds to the model ID. Therefore on our case it is set to whatever the $slug passed was. Now you may be asking, why should that still be a problem if we’re forcing the ID right before the save? Once again, let’s dive into the core, and see that when Model::invalidFields() (called by Model::validates()) is figuring out what rules to execute during validation, it does the following so it knows if the current record being validated is a new record, or an existing one:

$exists = $this->exists();

For performance reasons, Model::exists() will save internally a variable so the next time someone calls Model::exists() it doesn’t have to execute another COUNT, unless you force it (by passing true as a parameter to exists). Model::save() naturally needs to know if the record exists to figure out if it’s an INSERT or an UPDATE. How does it find out? You’ve guessed it: by calling exists:

$exists = $this->exists();

It is not forcing exists() to re-run the query, which means on our case Model::exists() will return the value found the last time it was executed, which was back when the model was being validated. At that time Model::$id was set to $slug, so naturally Model::exists() said this record doesn’t exist. Hence, Model::save() thinks now the record doesn’t exist, and the save gets executed as an INSERT.

Now don’t get all hasty and think this is a bug, or complain saying that Model::save() should force Model::exists() to refresh. Instead think that we may be missing the point: there’s another convention which says Model should know the real Model ID before a validation is performed. It is a convention for our own benefit: so we can specify certain validation rules to run only on update, create, or both. So we need to change our code and make sure that the right model ID is set before executing Model::validates():

class PostsController extends AppController {
	function edit($slug) {
		$post = $this->Post->findBySlug($slug);

		if (!empty($this->data)) {
			// Let's force the ID, just in case user gets funky ideas
			$this->data['Post']['id'] = $post['Post']['id'];

			$this->Post->set($this->data);
			if ($this->Post->validates()) {
				// Do something else we need to do before saving
				// ...

				// and now save
				$this->Post->save($this->data);
			}
		}
	}
}

Related posts:

  1. CakePHP 1.2 tip of the day: be mindful of Model::set
  2. Something to consider when saving HABTM records in CakePHP
  3. Pagination with custom find types in CakePHP


Leave a Comment

4 Comments to "CakePHP tip of the day: pay attention to conventions"

  1. Oct232007 at 9:43 pm

    sarimarton [Visitor] wrote:

    Great article, thank you!

  2. Oct252007 at 9:12 am

    Daniel Hofstetter [Visitor] wrote:

    Well, if the first example you described really does an insert instead of an update (I didn’t test it myself), then I have to say it is a bug. If I explicitly pass the id in the data I want to save, then Cake should use this id, and not assume something else.

    What you described is an unwanted side effect of the validates() function. If you omit calling this function, it would update the record, right? So the behavior of the save() function depends on whether I call validates() first. In one case the save() function will insert the data, in the other case it updates the record, even though in both cases the same data is provided. Hm, sounds like a bug to me ;-)

  3. Apr022009 at 5:10 pm

    Mark De Verno [Visitor] wrote:

    Nice article, but I agree with Daniel. As you are explicitly passing in the id of the data, Cake should update, not insert.

  4. Apr282010 at 12:36 pm

    Let them eat cake » UPDATE statt INSERT wrote:

    [...] allerdings nicht – Cake erstellt trotzdem stur neue Einträge. Eine Google Suche brachte die Lösung. Cake prüft die Existenz von Einträgen während des Validierens. Der richtige Callback für [...]

 
Powered by Wordpress and MySQL. Clauz's design for by Cricava