Skip to content
Permalink
Browse files

Do type checks when looking for models in Controller::$uses.

This solves issues with models not being added when $uses = true.

Fixes #3774
  • Loading branch information...
markstory committed Apr 19, 2013
1 parent 63b392a commit efd86a498afc19ada7ff01a2ee90cab7a5d3bbf3
Showing with 18 additions and 5 deletions.
  1. +1 −1 lib/Cake/Controller/Controller.php
  2. +17 −4 lib/Cake/Test/Case/Controller/ControllerTest.php
@@ -723,7 +723,7 @@ public function loadModel($modelClass = null, $id = null) {
}
$this->uses = ($this->uses) ? (array)$this->uses : array();
if (!in_array($modelClass, $this->uses)) {
if (!in_array($modelClass, $this->uses, true)) {
$this->uses[] = $modelClass;
}
@@ -447,11 +447,24 @@ public function testLoadModel() {
$result = $Controller->loadModel('ControllerPost');
$this->assertTrue($result);
$this->assertTrue(is_a($Controller->ControllerPost, 'ControllerPost'));
$this->assertTrue(in_array('ControllerPost', $Controller->uses));
$this->assertInstanceOf('ControllerPost', $Controller->ControllerPost);
$this->assertContains('ControllerPost', $Controller->uses);
}
ClassRegistry::flush();
unset($Controller);
/**
* Test loadModel() when uses = true.
*
* @return void
*/
public function testLoadModelUsesTrue() {
$request = new CakeRequest('controller_posts/index');
$response = $this->getMock('CakeResponse');
$Controller = new Controller($request, $response);
$Controller->uses = true;
$Controller->loadModel('ControllerPost');
$this->assertInstanceOf('ControllerPost', $Controller->ControllerPost);
$this->assertContains('ControllerPost', $Controller->uses);
}
/**

4 comments on commit efd86a4

@dereuromark

This comment has been minimized.

Copy link
Member

replied Apr 22, 2013

Something changed in the current master branch and broke existing apps.
I suspect this commit. But I could also be wrong and the issue could be further back..

If you got in your AppController:

public $uses = array('Country');

Until recently, all other controller's $this->modelClass was correctly the model class of this controller.
Or at least pagination worked correctly.
Now it's always "Country", as the app model uses declares it, and $this->paginate() without arguments breaks.

In my case a plugin controller (RatingsController extends RatingsAppController extends AppController).

Even a manual public $uses = array('Ratings.Rating'); does not fix this!

Maybe the controller should array_unshift its modelClass in app uses to avoid this BC breaking?

@lorenzo

This comment has been minimized.

Copy link
Member

replied Apr 22, 2013

@dereuromark I think that is correct. It should always be the first (or only) model you declare.

@ADmad

This comment has been minimized.

Copy link
Member

replied Apr 22, 2013

If $uses is defined in mergeParent the default $modelClass is added to $uses using array_unshift.

@markstory

This comment has been minimized.

Copy link
Member Author

replied Apr 22, 2013

I'm really confused as to how this change could have broken what you're describing. Would the following reproduce the issue?

class AppController extends Controller {
  public $uses = array('Country');
}

class TasksController extends AppController {
  function index() {
    $this->set('tasks', $this->paginate());
  }
}
Please sign in to comment.
You can’t perform that action at this time.