Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undesired notice in Model.php _prepareUpdateFields for counterCache #3469

Closed
cuppett opened this issue May 11, 2014 · 4 comments
Closed

Undesired notice in Model.php _prepareUpdateFields for counterCache #3469

cuppett opened this issue May 11, 2014 · 4 comments
Labels
Milestone

Comments

@cuppett
Copy link
Contributor

cuppett commented May 11, 2014

For some models, I aggregate associations programmatically through constructors and the class hierarchy like this:

    public function __construct($id = false, $table = null, $ds = null) {
        parent::__construct($id, $table, $ds);
        $this->belongsTo['Post'] = array('className' => 'Post', 
                    'foreignKey' => 'parent_id');
    }

This can lead to some noisy notice messages in Model.php:

Notice (8): Undefined index: counterCache [CORE\Cake\Model\Model.php, line 2109]

It can be worked around in the definition by including counterCache and setting false explicitly:

    public function __construct($id = false, $table = null, $ds = null) {
        parent::__construct($id, $table, $ds);
        $this->belongsTo['Post'] = array('className' => 'Post', 
                     'foreignKey' => 'parent_id', 'counterCache' => false);
    }

However, the framework could catch this more elegantly with an isset check:

    protected function _prepareUpdateFields($data) {
        $foreignKeys = array();
        foreach ($this->belongsTo as $assoc => $info) {
            if (isset($info['counterCache'] && $info['counterCache']) {
                $foreignKeys[$assoc] = $info['foreignKey'];
            }
        }
@dereuromark dereuromark added this to the 2.4.10 milestone May 11, 2014
@dereuromark
Copy link
Member

Did you try switching the order?

    $this->belongsTo['Post'] = ...
    parent::__construct($id, $table, $ds);

That would assert that the counterCache key as well as others are properly set up through the constructor - as it is meant to work.
You should not hook into it this way, IMO, using attributes directly (at least not without fully declaring them). It fails as expected.

If you need to do it manually afterwards, the bindModel() method would be the way to go.

@cuppett
Copy link
Contributor Author

cuppett commented May 11, 2014

Sure, let me give a couple things a try. Thanks!

@cuppett
Copy link
Contributor Author

cuppett commented May 11, 2014

Both those work. The parent constructors should fire first (in my opinion), but this style works nice too:

        parent::__construct($id, $table, $ds);
        $this->bindModel(array('belongsTo' =>
            array('Post' => array(
                'className' => 'Post', 'foreignKey' => 'parent_id')
            )
        ));

You can close or add the extra safety check anyway. Either works for me.

Thanks!

@dereuromark
Copy link
Member

Closing as there is an open PR to continue the discussion: #3470 .

Thank you for your quick reply. We will look into it.

markstory added a commit that referenced this issue May 12, 2014
Fixes #3469, explicit isset check for counterCache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants