kafoso / doctrine-firebird-driver

Firebird driver for the Doctrine DBAL/ORM
MIT License
14 stars 16 forks source link

Transaction + Generator #8

Closed christophergyn closed 4 years ago

christophergyn commented 4 years ago

Hello, Dear friend, I have a business class that saves twice, two records in the same table, so the generator should add twice. But it has only one more number. All in the same transaction. Example: generator = 234 register 235 save and 236 save, but generator show me 235. I have a problem when saving next register. duplicate primary key.

kafoso commented 4 years ago

I have not encountered any issues with generators myself. Transactions or not.

However, you will have to use savepoints with Firebird. This is achieved by calling #Connection->setNestTransactionsWithSavepoints(true).

If you are using Symfony, you can add a compiler pass to ensure this. Something like:

use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

class SetupDoctrinePass implements CompilerPassInterface
{
    public function process(ContainerBuilder $container)
    {
        $entityManager = $container->get('doctrine.orm.default_entity_manager');
        $entityManager->getConnection()->setNestTransactionsWithSavepoints(true);
    }
}

It's old syntax/code. May be different in Symfony 4.x.

What version of kafoso/doctrine-firebird-driver are you using?

Things to consider

Annotations could be incorrect

If you are using PHP annotations, it should look something like this:

use Doctrine\ORM\Mapping as ORM;

class Message
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="SEQUENCE")
     * @ORM\SequenceGenerator(sequenceName="message_seq", initialValue=1, allocationSize=100)
     */
    protected $id = null;
    //...
}

See: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/basic-mapping.html#sequence-generator

Could you please provide a sample snippet of your annotations?

Database trigger at fault

There may be a database trigger, which causes the conflict.

christophergyn commented 4 years ago

Thanks for atention;

I use: Zendframework: 3.0 kafoso/doctrine-firebird-driver: 2.6.2 Firebird 2.5 x86 PHP 7.2

namespace Application\Entity;

use Doctrine\ORM\Mapping as ORM;

/**
 * Pedidoit
 *
 * @ORM\Table(name="pedidoit", uniqueConstraints={@ORM\UniqueConstraint(name="UN_PRODPED", columns={"COD_PED", "COD_PROD"})})
 * @ORM\Entity
 * @ORM\Entity(repositoryClass="Application\Entity\PedidoitRepository")
 */
class Pedidoit extends AbstractEntity{
    /**
     * @var integer
     *
     * @ORM\Column(name="COD_PEDIT", type="integer", nullable=false)
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="SEQUENCE")
     * @ORM\SequenceGenerator(sequenceName="GE_IDPEDIDOIT", initialValue=1, allocationSize=100000) 
     */
    private $codPedit;

But the problem remains

Code where start Transaction.

try {
       $this->getEm()->getConnection()->setNestTransactionsWithSavepoints(true); 

       $this->getEm()->beginTransaction();

       $trocarPedidoProdutoComplementoService = $this->getEvent()->getApplication()->getServiceManager()->get(TrocarPedidoProdutoComplementoService::class);

       $pedido = $trocarPedidoProdutoComplementoService->save($data);

       $this->getEm()->commit();

} catch (\Exception $exc) {

       $this->getEm()->rollback();

       echo $exc->getMessage();

}
christophergyn commented 4 years ago

I dont have trigger...in my database.

kafoso commented 4 years ago

Thanks.

I can't really see what goes on inside your service class ($trocarPedidoProdutoComplementoService->save(...)). Thus, I don't know if you are persisting and flushing things correctly.

I've made a quick test and it does work for me.

SQL:

RECREATE TABLE SEQUENCEGENERATORTABLE (
    id INT NOT NULL,
    PRIMARY KEY(id)
);
CREATE SEQUENCE GEN_SEQUENCEGENERATORTABLE_ID;
ALTER SEQUENCE GEN_SEQUENCEGENERATORTABLE_ID RESTART WITH 0;

Doctrine Entity:

<?php
namespace Kafoso\DoctrineFirebirdDriver\Test\Resource\Entity;

use Doctrine\ORM\Mapping as ORM;

/**
 * @ORM\Entity
 * @ORM\Table(name="SEQUENCEGENERATORTABLE")
 */
class SequenceGeneratorEntity
{
    /**
     * @ORM\Column(type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="SEQUENCE")
     * @ORM\SequenceGenerator(sequenceName="GEN_SEQUENCEGENERATORTABLE_ID", initialValue=1, allocationSize=100000)
     */
    private $id = null;

    /**
     * @return null|int
     */
    public function getId()
    {
        return $this->id;
    }
}

PHP test:

...
    public function testCanPersistMultipleInTransaction()
    {
        $albumA = new Entity\SequenceGeneratorEntity;
        $albumB = new Entity\SequenceGeneratorEntity;
        $albumC = new Entity\SequenceGeneratorEntity;
        $this->_entityManager->beginTransaction();
        $this->_entityManager->persist($albumA);
        $this->_entityManager->persist($albumB);
        $this->_entityManager->persist($albumC);
        $this->_entityManager->flush();
        $this->_entityManager->commit();
        $rows = $this->_entityManager->getConnection()->fetchAll("SELECT * FROM SEQUENCEGENERATORTABLE");
        $expected = [
            ['ID' => 1],
            ['ID' => 2],
            ['ID' => 3],
        ];
        $this->assertSame($expected, $rows);
        $this->assertSame(1, $albumA->getId());
        $this->assertSame(2, $albumB->getId());
        $this->assertSame(3, $albumC->getId());
    }
...

Test passes.


A side note

$this->getEm()->beginTransaction(); should go before the try. If the transaction somehow fails to start, and you end up in the catch, you are now either:

  1. Causing another failure, because rollback has no transaction to working with. This means the exception, you catch, is for naught, because the rollback will cause a third, unintended exception.
  2. Rolling back a parent transaction, in case they are nested. This could be very bad.
christophergyn commented 4 years ago

Dear friend,

First of all I would like to thank you for "A side note" because you are absolutely right. I made the correction as measured.

With the test you presented me, I identified where the problem happens.

public function saveItens() {
        if (isset($this->dados['pedidosItem']) && (sizeof($this->dados['pedidosItem']) > 0)) {
            $savePedidoItService = new SavePedidoItService($this->getEm());
            foreach ($this->dados['pedidosItem'] as $arrayPedidoIt){
                $savePedidoItService->save($arrayPedidoIt);
            }            
        }
    }

Persistence Method

public function save(Array $data = array()){
        $entity = new $this->entity($data);
        $this->em->persist($entity);
        $this->em->flush();
        return $entity;
    }

Watching, I realized that I'm flushing every time I save. What is wrong. Then the correct would be.

public function saveItens() {
        if (isset($this->dados['pedidosItem']) && (sizeof($this->dados['pedidosItem']) > 0)) {
            $savePedidoItService = new SavePedidoItService($this->getEm());
            foreach ($this->dados['pedidosItem'] as $arrayPedidoIt){
                $savePedidoItService->save($arrayPedidoIt);              
            }  
            $this->getEm()->flush();           
        }
    }

Remove the flush from the persistence method.

public function save(Array $data = array()){
        $entity = new $this->entity($data);
        $this->em->persist($entity);
        return $entity;
    }

Amazing that the problem repeats with the generator.

I used

$this->em = $this->getEvent()->getApplication()->getServiceManager()->get(\Doctrine\ORM\EntityManager::class);

To inject EntityManager in my controlller.

What is the best practice to Inject?

kafoso commented 4 years ago

I see that you are flushing everything. I.e. no argument in the ->flush() call. I'm not sure that's what you want to do.

Could you try either of these two things and report back what you find:

1: Flush each entity separately

...
$entity = new MyEntity;
$em->persist($entity);
$em->flush($entity);
...

2: Flush all newly created entities by passing array

...
$entityA = new MyEntity;
$em->persist($entityA);
$entityB = new MyEntity;
$em->persist($entityB);
$em->flush([$entityA, $entityB]);
...
christophergyn commented 4 years ago

Dear Friend;

I appreciate your time and all the tips. But as for the generator problem I couldn't solve it. So I made a trigger to check the generator, and correct if necessary. This was the solution.

Thank you very mutch.