OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
870 stars 436 forks source link

Fix the bug on created_at in the customer_entity table on update #4070

Open sdecalom opened 4 months ago

sdecalom commented 4 months ago

This pull request can fix the bug on created_at in the customer_entity table on update.

Description (*)

Since MySQL 8.0, we have had a problem with the created_on column in the customer_entity table. In the beforeSave() function, we have the date in a timezone format ==> 2010-04-21T07:42:19+00:00. However, after the update, if your MySQL is set up with time_zone='+02:00', your created_at will not be in UTC.

So, the solution is to remove the timezone format. "2010-04-21T07:42:19+00:00" ==> "2010-04-21 07:42:19"

By doing this, the created_at field with time_zone=+2 we have a bad result

UPDATE customer_entity SET created_at = '2010-04-21T07:42:19+00:00' WHERE entity_id=5;
-- Result: 2010-04-21 09:42:19

By doing this, the created_at field will maintain the correct value.

UPDATE customer_entity SET created_at = '2010-04-21 07:42:19' WHERE entity_id=5;
-- Result: 2010-04-21 07:42:19

Related Pull Requests

Fixed Issues (if relevant)

  1. Fixes OpenMage/magento-lts#4069

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

Contribution checklist (*)

fballiano commented 4 months ago

I was trying to test this, but with or without this PR I always get the data saved in mysql wrong by 1h (9.15 is stored but it's 10.15 here now).

system timezone is set correctly and if I do SELECT NOW() I get the right time. timezone is also set correctly in openmage settings.

mmmm

pquerner commented 4 months ago

Anything you do in Magento is UTC by default. \Mage_Core_Model_App::_initEnvironment

fballiano commented 4 months ago

@pquerner ye ye I know but I live in the london timezone, but I recognize now that the can still be a difference between london and UTC, which I didn't think was possible

pquerner commented 4 months ago

But you meantioned you did a SELECT NOW() - which is outside of Magento. So it will most likely tell you your OS timezone. Or whatever you did tell MySQL what your timezone is. Just wanted to clarify.

fballiano commented 4 months ago

timezone in mysql is "SYSTEM"

kiatng commented 4 months ago

I tested with the following script:

        $object = Mage::getModel('customer/customer');
        $attribute = $object->getResource()->getAttribute('created_at');
        $now = Mage::getSingleton('core/date')->date('Y-m-d H:i:s'); // My locale time zone is +8:00 in OpenMage.

        // Test datetime value is null
        $attribute->getBackend()->beforeSave($object);
        $createdAt0 = $object->getCreatedAt(); // Expect UTC datetime

        // Test datetime value in local time zone
        $object->setCreatedAt($now);
        $attribute->getBackend()->beforeSave($object);
        $createdAt1 = $object->getCreatedAt(); // Expect convert $now to UTC datetime

        $r = [
            'now' => $now,
            'created_at0' => $createdAt0,
            'created_at1' => $createdAt1,
        ];

Result before PR:

 array(3) {
  ["now"] => string(19) "2024-07-07 20:47:03"
  ["created_at0"] => string(19) "2024-07-07 12:47:03"
  ["created_at1"] => string(25) "2024-07-07T12:47:03+00:00"
}

After PR:

 array(3) {
  ["now"] => string(19) "2024-07-07 21:03:12"
  ["created_at0"] => string(19) "2024-07-07 13:03:12"
  ["created_at1"] => string(19) "2024-07-07 13:03:12"
}

Datetime in UTC are set and ready to be saved to DB. So both sets of results seem correct to me.

kiatng commented 4 months ago

Ref earlier doc MySQL 5.7 has no time zone specification. But [MySQL 8.0] doc](https://dev.mysql.com/doc/refman/8.0/en/date-and-time-literals.html) has it

Beginning with MySQL 8.0.19, you can specify a time zone offset when inserting TIMESTAMP and DATETIME values into a table. The offset is appended to the time part of a datetime literal, with no intravening spaces, and uses the same format used for setting the time_zone system variable, with the following exceptions:

  • For hour values less than 10, a leading zero is required.
  • The value '-00:00' is rejected.
  • Time zone names such as 'EET' and 'Asia/Shanghai' cannot be used; 'SYSTEM' also cannot be used in this context.

If I interpret this correctly, before v8.0, time zone offset is ignore. After v8.0, MySQL will do conversion using the offset and the config param time_zone. You can see the conversion example in the doc. It has this snippet to check the time zone:

SELECT @@system_time_zone;`
# Result in my DB: +08

I do another test MySQL 8.0.37

Without setting created_at

        $object = Mage::getModel('customer/customer')
            ->setData([
                'firstname' => 'John',
                'lastname' => 'Doe',
                'email' => 'j@ex.com',
            ])
            ->save();
        print_r($object->getCreatedAt());

With created_at, for example during data import.

        $object = Mage::getModel('customer/customer')
            ->setData([
                'firstname' => 'John1',
                'lastname' => 'Doe',
                'email' => 'j1@ex.com',
                'created_at' => Mage::getSingleton('core/date')->date('Y-m-d H:i:s'),
            ])
            ->save();
        print_r($object->getCreatedAt());

Result Before PR: image

Result After PR: image

Conclusion: Both sets of result are correct, meaning this PR is not needed in my test environment.

@sdecalom Any idea what server params I miss?

sdecalom commented 4 months ago

@kiatng, any idea.

For my side in MySQL server I have this configuration image

In DB I have this image

In afterLoad() Value of date => 2021-03-17 19:58:47 image

Value of created_at => 2021-03-17T20:58:47+01:00 image

Value of created_at => 2021-03-17T19:58:47+00:00 in beforeSave() image

But in DB I have 2021-03-17 20:58:47 instead of 2021-03-17 19:58:47 image

For my example, we have a difference of one hour

kiatng commented 4 months ago

My review of @sdecalom screenshots:

Screenshot 1 @@system_time_zone is set to local time zone, same as what I have.

Screenshot 3 `afterLoad()` result is correct > Value of date => 2021-03-17 19:58:47 Comment: the initial value as stored in the DB, the time zone is UTC > Value of `created_at` => 2021-03-17T20:58:47+01:00 Comment: the value after converting to local time zone with the UTC offset +01, meaning your local datetime is _2021-03-17 20:58:47_. I confirmed the above is a correct result with Windows Copilot and Gemini AI. My prompt is > Given datetime "2021-03-17T20:58:47+08:00", what is my local time I have changed the UTC offset to my local time zone in the prompt.
Screenshot 4 `beforeSave()` result is correct > Value of created_at => 2021-03-17T19:58:47+00:00 in beforeSave() I am not familiar with the IDE, my guess is that the initial value is `$date = '2021-03-17T20:58:47+01:00'`. This is in local time zone. And the resulted value `$zendDate->getIso()` is '2021-03-17T19:58:47+00:00'. This is correct as we want to save datetime in UTC.

Screenshot 5 DB value is incorrect

But in DB I have 2021-03-17 20:58:47 instead of 2021-03-17 19:58:47

Conclusion: you are correct, the value in DB should be 2021-03-17 19:58:47. However, I cannot replicate this in my test:

    public function indexAction()
    {
        $var['date'] = '2021-03-17T20:58:47+08:00'; // Note my time zone is UTC +8
        $object = Mage::getModel('customer/customer')->load(18);
        $object->setCreatedAt($var['date'])->save();
        $var['after_saved'] = $object->getCreatedAt();
        $object->load($object->getId());
        $var['after_load'] = $object->getCreatedAt();
        Zend_Debug::dump($var);
    }

output:

array(3) {
  ["date"] => string(25) "2021-03-17T20:58:47+08:00"
  ["after_saved"] => string(25) "2021-03-17T12:58:47+00:00"
  ["after_load"] => string(25) "2021-03-17T20:58:47+08:00"
}

image

pquerner commented 4 months ago

Anyone checked what happens when you load customers via collection?

kiatng commented 4 months ago

Anyone checked what happens when you load customers via collection?

    public function indexAction()
    {
        $var['date'] = '2021-03-17T20:58:47+08:00';
        $object = Mage::getModel('customer/customer')
            ->getCollection()
            ->addAttributeToFilter('entity_id', 18)
            ->getFirstItem();
        //$object->setCreatedAt($var['date'])->save();
        //$var['after_saved'] = $object->getCreatedAt();
        //$object->load($object->getId());
        $var['after_load'] = $object->getCreatedAt();
        Zend_Debug::dump($var);
    }

result:

array(2) {
  ["date"] => string(25) "2021-03-17T20:58:47+08:00"
  ["after_load"] => string(19) "2021-03-17 12:58:47"
}

Mage_Eav_Model_Entity_Attribute_Backend_Time_Created::afterLoad() is skipped, datetime is UTC.