Auto-Increment option

[soullivaneuh]

trafficstars

It was already asked on #29 but closed by the author.

I think it might be very interesting to propose it as an option, if this is technically possible.

Assert can not currently be done on IDs.

Test case example:

public function testUserIsLoadedFromGithub()
{
    $this->userManager->updateUser(
        (new User())
            ->setGithubId(1337)
            ->setUsername('toto')
            ->setPlainPassword(uniqid())
            ->setEmail('[email protected]')
    );
    $this->userManager->updateUser(
        (new User())
            ->setGithubId(4242)
            ->setUsername('uCustom')
            ->setPlainPassword(uniqid())
            ->setEmail('[email protected]')
    );

    $user = $this->userProvider->loadUserByOAuthUserResponse(
        $this->createUserResponseMock(
            $this->container->get('hwi_oauth.resource_owner.github')
        )
    );
    static::assertSame(2, $user->getId());
    static::assertSame('uCustom', $user->getUsername());
    $this->assertUserAttributes($user, 'github');
}

Regards.

[dmaicher]

If I understand the test correctly: Can you not do this?

$user = (new User())
            ->setGithubId(4242)
            ->setUsername('uCustom')
            ->setPlainPassword(uniqid())
            ->setEmail('[email protected]')

$this->userManager->updateUser($user);

...

$loadedUser = $this->userProvider->loadUserByOAuthUserResponse(
    $this->createUserResponseMock(
        $this->container->get('hwi_oauth.resource_owner.github')
    )
);

static::assertSame($user->getId(), $loadedUser->getId());

I mean its hardly required to rely on hardcoded auto generated ID's in tests I believe.

[soullivaneuh]

I can do this, indeed. :-)

But still, the option can be interesting if it's technically possible without pain. :wink:

[aaa2000]

With postgresql and sequence strategy, this code in a custom WebTestCase seems to work

   /**
     * @before
     */
    protected function resetDatabaseSequences()
    {
        /** @var \Doctrine\ORM\EntityManager $em */
        $em = $this->getContainer()->get('doctrine')->getManager();
        /** @var \Doctrine\ORM\Mapping\ClassMetadata[] $metadatas */
        $metadatas = $em->getMetadataFactory()->getAllMetadata();
        foreach ($metadatas as $metadata) {
            if ($metadata->isIdGeneratorSequence()) {
                $em->getConnection()->executeQuery(sprintf(
                    'ALTER SEQUENCE %s RESTART',
                    $metadata->getSequenceName($em->getConnection()->getDatabasePlatform())
                ));
            }
        }
    }

[mnapoli]

Unfortunately with MySQL the same trick does not seem to be possible as ALTER TABLE triggers an implicit commit, which doesn't work with this package and triggers an error (#58).

If anyone has a trick to have predictable IDs I'm interested :)

[mnapoli]

A little update here: my workaround is to have a SQL script that resets the autoincrements.

Then this is what I do in my BeforeScenario in Behat:

    /**
     * @BeforeScenario
     */
    public function beforeScenario()
    {
        $defaultConnection = $this->doctrine->getConnection();
        // Reset the autoincrements
        $defaultConnection->exec(file_get_contents(__DIR__ . '/../../database/autoincrements.sql'));

        // The StaticDriver starts a transaction at the beginning, let's start from scratch because of the import above
        try {
            StaticDriver::commit();
        } catch (\Exception $e) {
            // There is a transaction only the first time
        }
        StaticDriver::beginTransaction();
    }

You'll notice the commit and try/catch: this is because on connection the StaticDriver auto-starts a transaction. That transaction is broken by the ALTER TABLE in autoincrements.sql, the only way I could make it work reliably was with this. Hope this helps because it took a lot of time to figure out ^^!

[dmaicher]

@mnapoli actually very similar to a workaround I have in some tests :see_no_evil:

For me there is no need to catch any exceptions though:

    public function setUp(): void
    {
        parent::setUp();
        StaticDriver::rollBack();
        $this->em->getConnection()->executeQuery('ALTER TABLE foo AUTO_INCREMENT = 1000');
        StaticDriver::beginTransaction();
    }

[thewholelifetolearn]

I used @aaa2000 solution and tweaked it a bit so that it works with fixtures too:

    public static function tearDownAfterClass() : void
    {
        static::createClient();
        /** @var $em EntityManager */
        $em = self::$container->get('doctrine.orm.default_entity_manager');
        $metadatas = $em->getMetadataFactory()->getAllMetadata();
        foreach ($metadatas as $metadata) {
            if ($metadata->isIdGeneratorSequence()) {
                $max = $em->getConnection()->fetchAll(sprintf(
                    'SELECT MAX(%s) FROM %s',
                    $metadata->getSingleIdentifierColumnName(),
                    $metadata->getTableName()
                    )
                );
                $max = (is_null($max[0]['max']) ? 1 : $max[0]['max'] + 1);
                $em->getConnection()->executeQuery(sprintf(
                    'ALTER SEQUENCE %s RESTART WITH %d',
                    $metadata->getSequenceName($em->getConnection()->getDatabasePlatform()),
                    $max
                    )
                );
            }
        }
    }

It works with PosstgreSQL 10.3.

[desmax]

You should not implement this :) My tests got better just because I couldn't rely on ids.

[mislavjakopovic]

+1 for implementing this.

Yes on new project you should write tests from the start properly, but when you work on a company project which has >1500 tests that heavily rely on ids, refactoring all of those in favor of using this bundle is simply not an option.

Anyways, here's a quite performant and dynamic workaround which might help someone in future (inspired by knowledge from above answers):

protected function setUp(): void
{
    $connection = $this->getContainer()->get('doctrine.dbal.default_connection');

    $results = $connection->executeQuery(
        'SELECT table_name FROM information_schema.tables WHERE table_schema = DATABASE() AND AUTO_INCREMENT > 1'
    )->fetchFirstColumn();

    foreach ($results as $result) {
        $connection->executeStatement('ALTER TABLE `' . $result . '` AUTO_INCREMENT = 1');
    }

    // The StaticDriver starts a transaction at the beginning, let's start from scratch because of the import above
    try {
        StaticDriver::commit();
    } catch (\Exception $e) {
        // There is a transaction only the first time
    }

    StaticDriver::beginTransaction();

    parent::setUp();
}

Before every test it will retrieve a list of tables which have been modified in last test (have AUTO_INCREMENT > 1) and issue reset just for those tables.