BC break: Calling a method unnecessarily

[thiagotalma]

Making an indirect call to the $this->getOldAttributes() method caused my code to break. I understand that the correct thing would be to reference $this->_oldAttributes.

Q	A
Is bugfix?	✔️
New feature?	❌
Breaks BC?	❌
Fixed issues	#19272

[bizley]

If I read this correctly here might be a problem when $this->_oldAttributes is null.

[thiagotalma]

yii2/framework/db/BaseActiveRecord.php

Line 642 in 2a7c6f2

if (isset($names[$name]) && (!array_key_exists($name, $this->_oldAttributes) || $this->isAttributeDirty($name, $value))) {

In theory, this check ensures that this problem will never occur.

[schmunk42]

caused my code to break.

What broke? Were there error messages, wrong data?

[yii-bot]

Thanks for posting in our issue tracker.
In order to properly assist you, we need additional information:

• When does the issue occur?
• What do you see?
• What was the expected result?
• Can you supply us with a stacktrace? (optional)
• Do you have exact code to reproduce it? Maybe a PHPUnit tests that fails? (optional)

Thanks!

This is an automated comment, triggered by adding the label status:need more info.

[thiagotalma]

The problem was because I was overriding the _get() method.

I only opened the PR because it may be that the same problem occurs with other people. In my understanding, it makes no sense to force the call in the $this->getOldAttributes method without the slightest need, since the $this->_oldAttributes is available.

Anyway, MY problem I already solved.
Feel free to close the PR. Or improve it, because, in my opinion, that wasn't the most adequate implementation. I'm open to suggestions.

[bizley]

I think we could change it to use private property indeed but with additional check for null.

[thiagotalma]

@bizley done

[samdark]

Would you please a line for CHANGELOG?

[thiagotalma]

@samdark done!

[bizley]

Sorry, just noticed that this is wrong, let's make it like that for example

private function isValueDifferent($newValue, $oldValue)
{
    if (is_array($newValue) && is_array($oldValue)) {
        $newValue = ArrayHelper::recursiveSort($newValue);
        $oldValue = ArrayHelper::recursiveSort($oldValue);
    }

    return $newValue !== $oldValue;
}

then we can just call it like

if (isset($names[$name]) && (!array_key_exists($name, $this->_oldAttributes) || $this->isValueDifferent($value, $this->_oldAttributes[$name]))) {
    $attributes[$name] = $value;
}

Otherwise we compare $value to an empty array if not set in old attributes.

[bizley]

Please modify as per my comment.

[samdark]

Thanks.

[samdark]

Owww... GitHub did not update the comments before I hit the merge button :(

[samdark]

Reverted commit for now. @thiagotalma would you please fix it as @bizley suggested?