Skip to content

Database Transaction Handling and ResourceModel, incompatible with outside transactions (and generally a mess?) #7597

Open
@ludwig-gramberg

Description

@ludwig-gramberg

Today I stumbled upon a weird design choice in Magento which makes using transaction problematic if not impossible:

When using Magentos Repositories, lets say, the CustomerRepository, it is wrapped in an transaction and at the end of that transaction the commit callbacks are being called.
The only problem being, that the callbacks are coupled with the resource model and its connections transaction. But what if I want to wrap multiple things in one transaction using the same connection?
Well in that case the commit callbacks wont fire because they do so only on commit level 0.
Shouldn't all the commit callbacks be coupled to the connection rather than the resource model?
Just isn't very (re)usable that way.
And digging deeper I found the CallbackPool class which uses static public methods, so anyone can mess with it... it holds the callbacks grouped by the connection instance. But it is only used inside the resource model commit method.

And how does using the repositories and the frameworks transaction object work together? I think not at all...

Preconditions

  • mage 2.1.2

Steps to reproduce

wrap the save of a repository inside another transaction which is called on the
Magento\Framework\DB\Adapter\AdapterInterface
for instance:

<?php
namespace Something;

use Magento\Customer\Model\ResourceModel\CustomerRepository;
use Magento\Framework\App\ResourceConnection;

class Something {

    public function __construct(
        CustomerRepository $customerRepository,
        ResourceConnection $connection
    ) {
        $connection = $connection->getConnection('write');

        $customer = $customerRepository->getById(1);
        $customer->setFirstname('New Firstname');

        $connection->beginTransaction();
        try {

            $customerRepository->save($customer);
            $connection->commit();

            // the callbacks are all still here ...
    
            $callbacks = CallbackPool::get(spl_object_hash($connection));
            foreach ($callbacks as $callback) {
                call_user_func($callback);
            }
            
        } catch(\Exception $e) {
            $connection->rollBack();
            throw $e;
        }

    }
}

Expected result

the commit callbacks run

Actual result

the commit callbacks don't run
because of: vendor/magento/framework/Model/ResourceModel/AbstractResource.php

public function commit()
    {
        $this->getConnection()->commit();
        /**
         * Process after commit callbacks
         */
        if ($this->getConnection()->getTransactionLevel() === 0) {
            $callbacks = CallbackPool::get(spl_object_hash($this->getConnection()));
            try {
                foreach ($callbacks as $callback) {
                    call_user_func($callback);
                }
            } catch (\Exception $e) {
                throw $e;
            }
        }
        return $this;
    }

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area: FrameworkComponent: DBIssue: ConfirmedGate 3 Passed. Manual verification of the issue completed. Issue is confirmedPriority: P2A defect with this priority could have functionality issues which are not to expectations.Progress: ready for devReproduced on 2.1.xThe issue has been reproduced on latest 2.1 releaseReproduced on 2.2.xThe issue has been reproduced on latest 2.2 releaseReproduced on 2.3.xThe issue has been reproduced on latest 2.3 releaseSeverity: S2Major restrictions or short-term circumventions are required until a fix is available.bug report

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions