Skip to content
This repository has been archived by the owner on Jan 6, 2022. It is now read-only.

Unable to set a value to NULL anymore since release 0.4.0 #12

Open
b-enoit-be opened this issue Nov 17, 2015 · 4 comments
Open

Unable to set a value to NULL anymore since release 0.4.0 #12

b-enoit-be opened this issue Nov 17, 2015 · 4 comments

Comments

@b-enoit-be
Copy link

For some fields, blanking the value is actually resulting as a NULL in the database and not an empty string, but since 0.4.0 NULL is considered as 'inherit from parent scope'.

This is not totally true in this case because the inheriting will then be done on the config.xml of the module, so it will side effect the value to be 90 when the intended purpose was to have it NULL, so disable the functionality (as the comment on the field is prompting it).

Module is from an EE version : Enterprise_Pci

Admin field
Admin field

Database state when the field is blanked
Database state when the field is blanked

Extract from app/code/core/Enterprise/Pci/etc/config.xml where we can see that, if the value is removed out of the database it will side effet to 90

<default>
    <admin>
        <security>
            <session_cookie_lifetime>900</session_cookie_lifetime>
            <lockout_failures>6</lockout_failures>
            <lockout_threshold>30</lockout_threshold>
            <password_lifetime>90</password_lifetime>
            <password_is_forced>1</password_is_forced>
        </security>
    </admin>
</default>

And here is how this value is actually used in the Observer of the module :

return 86400 * (int)Mage::getStoreConfig('admin/security/password_lifetime');
@punkstar
Copy link
Owner

@benn-be Do you have an excerpt from your yaml file that you could share to demonstrate this issue?

@b-enoit-be
Copy link
Author

The command I launch :

../vendor/bin/mageconfigsync --env=preview ../config/config.yaml

config.yaml

'magento_ee-1.14.1.0':
    default: &magento_ee_stock
        #some values reduced because they don't matter
prod:
    default: &prod_default
        <<: *magento_ee_stock
        admin/security/password_lifetime: null
preprod:
    default: &preprod_default
        <<: * prod_default
preview:
    default: &preview_default
        <<: *preprod_default
        #some values reduced because they don't matter

Expected result :
Setting admin/security/password_lifetime of default to null but from 0.4.0 on, this is actually removing the entry admin/security/password_lifetime from core_config_data.

That was what I actually expected for some values (to be able to recheck the use default checkbox in the admin), but since the basic assomption that null is not a valid value for a field in core_config_data is untrue, this does not work correctly then.

I'm not sure how we could overcome that matter, I tried to change LoadCommand.php to find a right way to do it but I was not able to find a clever solution.

The lines involved are actually those :

if ($value !== null) {
    $affected_rows = $config_adapter->setValue($path, $value, $scope_data['scope'], $scope_data['scope_id']);
} else {
    $affected_rows = $config_adapter->deleteValue($path, $scope_data['scope'], $scope_data['scope_id']);
}

@b-enoit-be
Copy link
Author

@punkstar Follow up information, if I blank any field on Magento config, it sets it to null.

So maybe a fix could be

if ($value === '') {
    $affected_rows = $config_adapter->setValue($path, null, $scope_data['scope'], $scope_data['scope_id']);
} elseif ($value !== null) {
    $affected_rows = $config_adapter->setValue($path, $value, $scope_data['scope'], $scope_data['scope_id']);
} else {
    $affected_rows = $config_adapter->deleteValue($path, $scope_data['scope'], $scope_data['scope_id']);
}

I'm currently looking into Mage core to find evidence that a field set to blank in the admin is automatically translated to null.

@b-enoit-be
Copy link
Author

I can confirm the above statement.

Mage_Core_Model_Resource_Db_Abstract::save is calling _prepareDataForSave
Mage_Core_Model_Resource_Db_Abstract:: _prepareDataForSave is calling _prepareDataForTable
Mage_Core_Model_Resource_Abstract::_prepareDataForTable is calling prepareColumnValue on write adapter
Varien_Db_Adapter_Pdo_Mysql::prepareColumnValue states

    switch ($column['DATA_TYPE']) {
        // cases we don't care about

        case 'varchar':
        case 'mediumtext':
        case 'text':
        case 'longtext':
            $value  = (string)$value;
            if ($column['NULLABLE'] && $value == '') {
                $value = null;
            }
            break;

        // cases we don't care about
    }

    return $value;

app/code/core/Mage/Core/sql/core_setup/install-1.6.0.0.php

    ->addColumn('value', Varien_Db_Ddl_Table::TYPE_TEXT, '64k', array(), 'Config Value')

So the field is indeed nullable and a text, it is then reassigned to null by the above Varien_Db_Adapter_Pdo_Mysql::prepareColumnValue.

So if you set a field to blank in core_config_data Magento ORM will 'cast' it to null.

Database schema :
Database schema of core_config_data

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants