Fixed undefined array key "values" in Mage_Catalog_Model_Product_Attribute_Backend_Media

[S0FTWEX]

There were still some warnings after product import without providing images additional data.

fixed undefined array key "values" in /app/code/core/Mage/Catalog/Model/Product/Attribute/Backend/Media.php on line 113
and
fixed foreach() argument must be of type array|object, null given in /app/code/core/Mage/Catalog/Model/Product/Attribute/Backend/Media.php on line 167.

[alexh-swdev]

I have a simmilar PR pending, array_key_exists and if required is_null is prefererred over isset...

#4121 (comment)

[S0FTWEX]

I just followed the rest of the code (see few lines of code above where isset is used as well)

[sreichel]

Prior to php7.4 it was far slower.

I'd not follow it anymore.

Have a read ...

• https://dev.to/aleksikauppila/using-isset-and-empty-hurts-your-code-aaa

Same for empty()

• https://dev.to/klnjmm/never-use-empty-function-in-php-4pb0
• https://localheinz.com/articles/2023/05/10/avoiding-empty-in-php/

[sreichel]

May I ask where this error comes from?

Seems to be 3rd-party code. (?)

[S0FTWEX]

May I ask where this error comes from?

Seems to be 3rd-party code. (?)

Yes, errors may come from 3rd-party code, the old module CommerceExtensions_Productimportexport for importing products via Dataflow Profiles.

[sreichel]

Can you post a stacktrace and maybe share it?

However. Adding checks for existing array-key does not harm.

[S0FTWEX]

Can you post a stacktrace and maybe share it?

However. Adding checks for existing array-key to harm.

Errors are in system.log. There are only these warnings there (repeating many times), but without stacktrace :-(

[empiricompany]

Could you please fill out the template we use for PRs?
Specifically, I don't know the steps to reproduce the issues.

[alexh-swdev]

Can you post a stacktrace and maybe share it?
However. Adding checks for existing array-key to harm.

Errors are in system.log. There are only these warnings there (repeating many times), but without stacktrace :-(

You could temporarily add a "debug trap" like so:

if (!array_key_exists('value', $option)) {
    $lbl = array_key_exists('label', $option) ? $option['label'] : "-unknown-";
    Mage::log(sprintf("Value does not exist for '%s': %s", $lbl, print_r(debug_backtrace(2), true)), Zend_Log::ALERT);
}

In my above mentioned PR, the issue is coming from an old unused ama pay plugin. But nevertheless, imho the OM core should be able to handle such things in a proper way

[S0FTWEX]

Here is the result of the "debug trap". Softwex_Core_Model_Convert_Adapter_Productimport is my custom profile for importing products via Dataflow Advanced Profiles.

Array
(
    [0] => Array
        (
            [file] => /app/code/core/Mage/Eav/Model/Entity/Abstract.php
            [line] => 640
            [function] => beforeSave
            [class] => Mage_Catalog_Model_Product_Attribute_Backend_Media
            [type] => ->
        )

    [1] => Array
        (
            [file] => /app/code/core/Mage/Eav/Model/Entity/Abstract.php
            [line] => 1628
            [function] => walkAttributes
            [class] => Mage_Eav_Model_Entity_Abstract
            [type] => ->
        )

    [2] => Array
        (
            [file] => /app/code/core/Mage/Catalog/Model/Resource/Product.php
            [line] => 167
            [function] => _beforeSave
            [class] => Mage_Eav_Model_Entity_Abstract
            [type] => ->
        )

    [3] => Array
        (
            [file] => /app/code/core/Mage/Eav/Model/Entity/Abstract.php
            [line] => 1090
            [function] => _beforeSave
            [class] => Mage_Catalog_Model_Resource_Product
            [type] => ->
        )

    [4] => Array
        (
            [file] => /app/code/core/Mage/Core/Model/Abstract.php
            [line] => 379
            [function] => save
            [class] => Mage_Eav_Model_Entity_Abstract
            [type] => ->
        )

    [5] => Array
        (
            [file] => /app/code/local/Softwex/Core/Model/Convert/Adapter/Productimport.php
            [line] => 1121
            [function] => save
            [class] => Mage_Core_Model_Abstract
            [type] => ->
        )

    [6] => Array
        (
            [file] => /app/code/core/Mage/Adminhtml/controllers/System/Convert/ProfileController.php
            [line] => 252
            [function] => saveRow
            [class] => Softwex_Core_Model_Convert_Adapter_Productimport
            [type] => ->
        )

    [7] => Array
        (
            [file] => /app/code/core/Mage/Core/Controller/Varien/Action.php
            [line] => 424
            [function] => batchRunAction
            [class] => Mage_Adminhtml_System_Convert_ProfileController
            [type] => ->
        )

    [8] => Array
        (
            [file] => /app/code/local/Mage/Core/Controller/Varien/Router/Standard.php
            [line] => 262
            [function] => dispatch
            [class] => Mage_Core_Controller_Varien_Action
            [type] => ->
        )

    [9] => Array
        (
            [file] => /app/code/core/Mage/Core/Controller/Varien/Front.php
            [line] => 181
            [function] => match
            [class] => Mage_Core_Controller_Varien_Router_Standard
            [type] => ->
        )

    [10] => Array
        (
            [file] => /app/code/core/Mage/Core/Model/App.php
            [line] => 358
            [function] => dispatch
            [class] => Mage_Core_Controller_Varien_Front
            [type] => ->
        )

    [11] => Array
        (
            [file] => /app/Mage.php
            [line] => 737
            [function] => run
            [class] => Mage_Core_Model_App
            [type] => ->
        )

    [12] => Array
        (
            [file] => /index.php
            [line] => 114
            [function] => run
            [class] => Mage
            [type] => ::
        )
)

IMHO, I agree with @alexh-swdev, OpenMage core should be able to handle such things in a proper way...

[sreichel]

OpenMage core should be able to handle such things in a proper way

Imho checks for existing keys should be covered by OM.

[kiatng]

LGTM

[sreichel]

I have a simmilar PR pending, array_key_exists and if required is_null is prefererred over isset...

#4121 (comment)

I think it is bit different there, b/c there are additional checks to make sure the array-value is not null.

$value['values'] = null;

if (!is_array($value['values']) && strlen($value['values']) > 0) {
}

In this case we would pass null to strlen, that also deprecated.

[sreichel]

Tested.