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

Added the ability to extend StaticDriver for greater flexibility #291

Closed
wants to merge 1 commit into from

Conversation

prohalexey
Copy link

@prohalexey prohalexey commented Apr 25, 2024

Hi!
I use this package to run tests, but have some troubles with it.

I have 2 connections to different databases in the production. Master server had one database and local servers that have their own databases and database replicated from master server. Doctrine have 2 connections. For the tests I want to have 1 connection to two local databases without replication. But they have different SHA hash string, and I cannot see transactions that I made with first database on the second connenction.

With this changes I will be able to extend StaticDriver and get hash from only a few parameters, for example:

json_encode([
       'driver'        => $params['driver'],
       'host'          => $params['host'],
       'port'          => $params['port'],
       'user'          => $params['user'],
       'password'      => $params['password'],
       'dbname'        => $params['dbname'],
       'driverOptions' => $params['driverOptions'],
])

@dmaicher
Copy link
Owner

How exactly does your doctrine dbal config look like for the test environment?

@prohalexey
Copy link
Author

doctrine:
    dbal:
        default_connection: default
        connections:
            default:
                url: '%env(DATABASE_URL)%'
                server_version: "8.0.31"
                use_savepoints: true
                options:
                    # some TLS settings
            master:
                url: '%env(MASTER_DATABASE_URL)%'
                server_version: "8.0.31"
                use_savepoints: true
                options:
                    # some TLS settings
                replicas:
                    replica1:
                        url: '%env(DATABASE_URL)%'
                mapping_types:
                    enum: string

@dmaicher
Copy link
Owner

You mentioned

For the tests I want to have 1 connection to two local databases without replication

But that is not what you have configured? You have

  • default with env(DATABASE_URL)
  • master with primay env(MASTER_DATABASE_URL) and replica env(DATABASE_URL)

So the default and read replica connection are using the exact same database? Interesting setup 🤔

@prohalexey
Copy link
Author

Production:
Code reads and writes to the LOCAL DB and MASTER GLOBAL SERVER and read from REPLICA DB FROM MASTER

       ---                    |----MASTER GLOBAL SERVER---|         ---
       |                      |       A data center       |           |

 |--------LOCAL SERVER--------|                          |--------LOCAL SERVER--------|
 | B data center              |                          | C data center              |
 | 1. LOCAL DB                |                          | 1. LOCAL DB                |
 | 2. REPLICA DB FROM MASTER  |                          | 2. REPLICA DB FROM MASTER  |

Test: 2 connections with the same connection params to the 1 database

@dmaicher
Copy link
Owner

dmaicher commented Apr 25, 2024

In the end this is related to #289

I'm not sure how to fix this properly yet. I don't see StaticDriver as an extension point really 😕

@prohalexey
Copy link
Author

prohalexey commented Apr 25, 2024

Now I wrote a copy of your StaticDriver with changes in param hash and did

class ConnectionIsolationBreakerMiddleware implements Middleware
{
    public function wrap(Driver $driver): Driver
    {
        return new ExtendedStaticDriver($driver);
    }
}

And add into services_test.yaml

    # Removing isolation between default and master connections
    doctrine.logging.middleware.connection_isolation_breaker:
        class: Tests\ConnectionIsolationBreakerMiddleware

And set keep static connection to the true in the bootstrap.php(phpunit)

ExtendedStaticDriver::setKeepStaticConnections(true);

@prohalexey
Copy link
Author

But this is not the best way, because I have to rewrite this every time you release :))

@dmaicher
Copy link
Owner

See #289 (comment)

I think this should solve such problems

@prohalexey
Copy link
Author

@dmaicher Yes, closing this PR.

@prohalexey prohalexey closed this May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants