Skip to content

Commit

Permalink
Do type checks when looking for models in Controller::$uses.
Browse files Browse the repository at this point in the history 
This solves issues with models not being added when $uses = true.

Fixes #3774
  • Loading branch information
@markstory
markstory committed Apr 19, 2013
1 parent 63b392a commit efd86a4
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 5 deletions.
2 changes: 1 addition & 1 deletion lib/Cake/Controller/Controller.php
View file
Expand Up @@ -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;
}

Expand Down
21 changes: 17 additions & 4 deletions lib/Cake/Test/Case/Controller/ControllerTest.php
View file
Expand Up @@ -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);
}

/**
Expand Down

4 comments on commit efd86a4

@dereuromark
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@ADmad
Copy link
Member

@ADmad ADmad commented on efd86a4 Apr 22, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@markstory
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.