Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable cached fixtures directory #118

Open
mente opened this issue May 27, 2021 · 3 comments
Open

Configurable cached fixtures directory #118

mente opened this issue May 27, 2021 · 3 comments
Labels
enhancement New feature or request

Comments

@mente
Copy link

mente commented May 27, 2021

Is your feature request related to a problem? Please describe.
We've switched from custom implementation to this library to leverage fixtures caching (thanks for extraction from liip/LiipFunctionalTestBundle!). Also we use paratest to speed up our sqlite tests execution. To guarantee that the used database is different for each thread, we use TEST_TOKEN provided by paratest and connection factory similar to the one provided in https://github.com/liip/LiipTestFixturesBundle/blob/2.x/src/Factory/ConnectionFactory.php (btw it looks broken now as getDbNameFromEnv is not used)

Unfortunately, we still end up in concurrency problems but in fixtures backup usage, being it loading or restoring from fixtures. In our custom implementation we solved it with similar to connection factory - use different folders for different threads. It's possible to maintain this behavior by overwriting service definition of \Liip\TestFixturesBundle\Services\DatabaseBackup\SqliteDatabaseBackup but will appreciate if configurability of cache folder makes it to upstream

Describe the solution you'd like
\Liip\TestFixturesBundle\Services\DatabaseBackup\SqliteDatabaseBackup::getBackupFilePath cache folder (for other providers it's the same) should be configurable and instead of using $this->container->getParameter('kernel.cache_dir') allow configuration to come from bundle configuration.

For backwards-compatibility this parameter can be %kernel.cache_dir% by default.

Describe alternatives you've considered
Alternatives are to keep overwritten service definition on our side, but it's prone to future breaking changes.

Additional context
I've tried to add more configuration to liip_test_fixtures.cache_db but it accepts only service id, so I don't see clear non-breaking change here. Looking for alternatives and up to implement it f it satisfies both maintainers and my use case.

@alexislefebvre
Copy link
Collaborator

alexislefebvre commented May 30, 2021

Thanks for the detailed issue.

The support of paratest was removed to ease the maintenance of this bundle in #56, but we can restore it.

So we have to be able to:

  1. define a custom name for the database
  2. define a custom path for the backup

Could you please show the code that work for you today?

@mente
Copy link
Author

mente commented May 31, 2021

Thanks for looking into this.

We use the ConnectionFactory from the old times of the bundle (even has old reference to LiipFunctionalTestBundle):

use Doctrine\Bundle\DoctrineBundle\ConnectionFactory as BaseConnectionFactory;
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Configuration;

/**
 * Creates a connection taking the db name from the env with
 * a unique number defined by current process ID.
 *
 * @link https://github.com/liip/LiipFunctionalTestBundle/blob/master/Factory/ConnectionFactory.php idea borrowed here
 */
class DBALConnectionFactory extends BaseConnectionFactory
{
    /**
     * Create a connection by name.
     *
     * @param array         $params
     * @param Configuration $config
     * @param EventManager  $eventManager
     * @param array         $mappingTypes
     *
     * @return \Doctrine\DBAL\Connection
     */
    public function createConnection(array $params, Configuration $config = null, EventManager $eventManager = null, array $mappingTypes = array())
    {
        $parallelPrefix = getenv('TEST_TOKEN');
        if ($params['driver'] === 'pdo_sqlite') {
            $params['path'] = str_replace('__DBNAME__', $parallelPrefix, $params['path']);
        } else {
            $params['dbname'] = str_replace('__DBNAME__', $parallelPrefix, $params['dbname']);
        }

        return parent::createConnection($params, $config, $eventManager, $mappingTypes);
    }
}

And the one we use for sqlite fixtures replacement

use Doctrine\Common\DataFixtures\Executor\AbstractExecutor;
use Doctrine\DBAL\Connection;
use Doctrine\ORM\EntityManager;

/**
 * @author Aleksey Tupichenkov <[email protected]>
 */
final class SqliteDatabaseBackup extends AbstractDatabaseBackup implements DatabaseBackupInterface
{
    public function getBackupFilePath(): string
    {
        $token = getenv('TEST_TOKEN') ?: 'default';

        return $this->container->getParameter('kernel.cache_dir').'/test_sqlite_'. $token . md5(serialize($this->metadatas).serialize($this->classNames)).'.db';
    }

    private function getDatabaseName(Connection $connection): string
    {
        $params = $connection->getParams();
        if (isset($params['master'])) {
            $params = $params['master'];
        }

        $name = $params['path'] ?? ($params['dbname'] ?? false);
        if (!$name) {
            throw new \InvalidArgumentException("Connection does not contain a 'path' or 'dbname' parameter and cannot be dropped.");
        }

        return $name;
    }

    public function isBackupActual(): bool
    {
        $backupDBFileName = $this->getBackupFilePath();
        $backupReferenceFileName = $backupDBFileName.'.ser';

        return file_exists($backupDBFileName) && file_exists($backupReferenceFileName) && $this->isBackupUpToDate($backupDBFileName);
    }

    public function backup(AbstractExecutor $executor): void
    {
        /** @var EntityManager $em */
        $em = $executor->getReferenceRepository()->getManager();
        $connection = $em->getConnection();

        $executor->getReferenceRepository()->save($this->getBackupFilePath());
        copy($this->getDatabaseName($connection), $this->getBackupFilePath());
    }

    public function restore(AbstractExecutor $executor, array $excludedTables = []): void
    {
        /** @var EntityManager $em */
        $em = $executor->getReferenceRepository()->getManager();
        $connection = $em->getConnection();

        copy($this->getBackupFilePath(), $this->getDatabaseName($connection));
        $executor->getReferenceRepository()->load($this->getBackupFilePath());
    }
}

Had to copy-paste class because it's final in the bundle. Need to note that we're using latest 1.x version due to older symfony dependencies.

@alexislefebvre alexislefebvre added the enhancement New feature or request label Oct 4, 2022
@ndench
Copy link
Contributor

ndench commented Nov 30, 2023

This bundle now seems to support paratest out of the box? Except I'm still getting this issue with caching the sqlite database.
Following @mente's example resolves the issue. The below is the only change required to the SqliteDatabaseBackup class.

Would it be worth me submitting a PR for this?

    public function getBackupFilePath(): string
    {
+        $token = \getenv('TEST_TOKEN') ?: 'default';
+        return $this->container->getParameter('kernel.cache_dir').'/test_sqlite_'.$token.\md5(\serialize($this->metadatas).\serialize($this->classNames)).'.db';
-        return $this->container->getParameter('kernel.cache_dir').'/test_sqlite_'.md5(serialize($this->metadatas).serialize($this->classNames)).'.db';
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants