refactor: keep the snapshot bookkeeping and option reading small · DBDiff/DBDiff@3b6871a · GitHub
Skip to content

Commit 3b6871a

Browse files
jasdeepkhalsaclaude
jasdeepkhalsa
and
claude
committed
refactor: keep the snapshot bookkeeping and option reading small
Each side's snapshot is now kept by a class of its own rather than by DBManager, and a side's connection config by ServerSettings; MySQL's column comparison is in MySQLColumns, and the plain CLI options are read in a loop. No change in behaviour. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9659c8e commit 3b6871a

10 files changed

Lines changed: 116 additions & 105 deletions

‎src/DB/Adapters/MySQLAdapter.php‎

Lines changed: 0 additions & 12 deletions

‎src/DB/DBManager.php‎

Lines changed: 8 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<?php namespace DBDiff\DB;
22

33
use DBDiff\DB\Support\PostgresSessionOptions;
4-
use DBDiff\DB\Support\PostgresSnapshot;
4+
use DBDiff\DB\Support\PostgresSnapshotSet;
55
use DBDiff\DB\Support\PostgresPeers;
66
use DBDiff\DB\Support\ServerSettings;
77

@@ -24,11 +24,12 @@ class DBManager {
2424
/** The schema useSchema() last pointed the connections at. */
2525
protected ?string $schema = null;
2626

27-
/** @var array<string, string> each side's PostgreSQL snapshot, which its connections read as of */
28-
protected array $snapshots = [];
27+
/** The PostgreSQL snapshot each side is read as of. */
28+
public readonly PostgresSnapshotSet $snapshots;
2929

3030
function __construct() {
31-
$this->capsule = new Capsule;
31+
$this->capsule = new Capsule;
32+
$this->snapshots = new PostgresSnapshotSet;
3233
$dispatcher = new Dispatcher();
3334
$dispatcher->listen(StatementPrepared::class, function ($event) {
3435
$event->statement->setFetchMode(\PDO::FETCH_ASSOC);
@@ -52,27 +53,13 @@ public function connect($params): void {
5253
continue;
5354
}
5455

55-
// SQLite uses the db value as a file path; no server block required.
56-
if ($this->driver === 'sqlite') {
57-
$config = $this->adapter->buildConnectionConfig([], $input['db']);
58-
} else {
59-
$server = $params->{$input['server']};
60-
ServerSettings::requireFor($server, $input['server']);
61-
// --supabase / --sslmode supply an sslmode where the server's own
62-
// URL does not name one.
63-
if (isset($params->sslmode) && empty($server['sslmode'])) {
64-
$server['sslmode'] = $params->sslmode;
65-
}
66-
$config = $this->adapter->buildConnectionConfig($server, $input['db']);
67-
}
68-
56+
$config = ServerSettings::connectionConfig($this->adapter, $this->driver, $params, $input);
6957
$this->capsule->addConnection($config, $key);
7058
// The URL's session settings, on a connection just created.
7159
PostgresSessionOptions::apply($this->capsule->getConnection($key));
7260

7361
if ($this->driver === 'pgsql' && ($params->snapshot ?? true)) {
74-
$given = $key === 'source' ? ($params->sourceSnapshot ?? null) : ($params->targetSnapshot ?? null);
75-
$given ? $this->joinSnapshot($key, $given) : $this->holdSnapshot($key, $config);
62+
$this->snapshots->take($this->capsule, $key, $config, $params);
7663
}
7764
}
7865
if ($this->driver === 'pgsql') {
@@ -83,33 +70,6 @@ public function connect($params): void {
8370
}
8471
}
8572

86-
/**
87-
* Read `$side` as of a snapshot another session exported and holds — a
88-
* tool running DBDiff as one part of a larger read, so all of it sees the
89-
* same instant.
90-
*/
91-
private function joinSnapshot(string $side, string $id): void {
92-
if (PostgresSnapshot::join($this->getDB($side), $id, $side)) {
93-
$this->snapshots[$side] = $id;
94-
}
95-
}
96-
97-
/**
98-
* Read `$side` as of one instant: a second connection exports its
99-
* snapshot and keeps it open, and the one DBDiff reads with joins it —
100-
* see PostgresSnapshot.
101-
*/
102-
private function holdSnapshot(string $side, array $config): void {
103-
$anchor = $side . '__snapshot';
104-
$this->capsule->addConnection($config, $anchor);
105-
$connection = $this->capsule->getConnection($anchor);
106-
PostgresSessionOptions::apply($connection);
107-
$id = PostgresSnapshot::export($connection, $side);
108-
if ($id !== null && PostgresSnapshot::join($this->getDB($side), $id, $side)) {
109-
$this->snapshots[$side] = $id;
110-
}
111-
}
112-
11373
/**
11474
* Point both connections' catalog reads at one PostgreSQL schema.
11575
*
@@ -130,11 +90,7 @@ public function useSchema(string $schema): void {
13090
$this->capsule->getDatabaseManager()->purge($name);
13191
$this->capsule->addConnection($config, $name);
13292
PostgresSessionOptions::apply($this->capsule->getConnection($name));
133-
// The new connection reads as of the same instant as the last.
134-
if (isset($this->snapshots[$name])
135-
&& !PostgresSnapshot::join($this->getDB($name), $this->snapshots[$name], $name)) {
136-
unset($this->snapshots[$name]);
137-
}
93+
$this->snapshots->rejoin($this->getDB($name), $name);
13894
}
13995
}
14096

@@ -172,11 +128,6 @@ public function getAdapter(): DBAdapterInterface {
172128
return $this->adapter;
173129
}
174130

175-
/** Whether `$side` is read as of one snapshot. */
176-
public function readsSnapshot(string $side): bool {
177-
return isset($this->snapshots[$side]);
178-
}
179-
180131
public function getDriver(): string {
181132
return $this->driver;
182133
}

‎src/DB/Schema/TableSchema.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<?php namespace DBDiff\DB\Schema;
22

3-
use DBDiff\DB\Adapters\MySQLAdapter;
3+
use DBDiff\DB\Support\MySQLColumns;
44

55
use Diff\Differ\MapDiffer;
66
use Diff\Differ\ListDiffer;
@@ -109,7 +109,7 @@ public function getDiff(string $table, ?array $sourceSchema = null, ?array $targ
109109

110110
$differ = new MapDiffer();
111111
$diffs = $driver === 'mysql'
112-
? self::comparedAs([MySQLAdapter::class, 'comparableColumnDef'], $differ, $targetColumns, $sourceColumns)
112+
? self::comparedAs([MySQLColumns::class, 'comparable'], $differ, $targetColumns, $sourceColumns)
113113
: $differ->doDiff($targetColumns, $sourceColumns);
114114

115115
// A column renamed, not dropped and added: its data stays.

‎src/DB/Support/MySQLColumns.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php namespace DBDiff\DB\Support;
2+
3+
/** MySQL column definitions, as compared. */
4+
final class MySQLColumns {
5+
6+
/**
7+
* A column definition as compared, not as written. A collation names its
8+
* character set, and MySQL prints the set beside it or not depending on
9+
* how the column came to have it: `COLLATE utf8mb4_unicode_ci` on one
10+
* that took the table's, `CHARACTER SET utf8mb4 COLLATE
11+
* utf8mb4_unicode_ci` once it was altered. Read apart, the CHANGE that
12+
* set it was offered again after every run.
13+
*/
14+
public static function comparable(string $def): string {
15+
return preg_replace('/\bCHARACTER SET (\w+) (COLLATE \1_\w+)/i', '$2', $def);
16+
}
17+
}

‎src/DB/Support/PostgresSnapshot.php‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,7 @@ public static function scratch(\PDO $pdo, callable $work): mixed {
8787
try {
8888
return $work();
8989
} finally {
90-
$pdo->exec('ROLLBACK TO SAVEPOINT ' . self::SCRATCH);
91-
$pdo->exec('RELEASE SAVEPOINT ' . self::SCRATCH);
90+
self::undo($pdo, self::SCRATCH);
9291
}
9392
}
9493

@@ -105,14 +104,19 @@ public static function attempt(Connection $connection, callable $read): mixed {
105104
try {
106105
$result = $read();
107106
} catch (\Throwable $e) {
108-
$pdo->exec('ROLLBACK TO SAVEPOINT ' . self::ATTEMPT);
109-
$pdo->exec('RELEASE SAVEPOINT ' . self::ATTEMPT);
107+
self::undo($pdo, self::ATTEMPT);
110108
throw $e;
111109
}
112110
$pdo->exec('RELEASE SAVEPOINT ' . self::ATTEMPT);
113111
return $result;
114112
}
115113

114+
/** Back to the savepoint, and let it go. */
115+
private static function undo(\PDO $pdo, string $savepoint): void {
116+
$pdo->exec("ROLLBACK TO SAVEPOINT $savepoint");
117+
$pdo->exec("RELEASE SAVEPOINT $savepoint");
118+
}
119+
116120
private static function firstLine(\Throwable $e): string {
117121
return strtok($e->getMessage(), "\n") ?: get_class($e);
118122
}

‎src/DB/Support/PostgresSnapshotSet.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php namespace DBDiff\DB\Support;
2+
3+
use Illuminate\Database\Capsule\Manager as Capsule;
4+
use Illuminate\Database\Connection;
5+
6+
/**
7+
* The snapshot each side of a diff is read as of — see PostgresSnapshot —
8+
* and the connections that hold them.
9+
*/
10+
final class PostgresSnapshotSet {
11+
12+
/** @var array<string, string> by side */
13+
private array $ids = [];
14+
15+
/**
16+
* Read `$side`'s connection as of a snapshot: the one another session
17+
* exported and holds, when the params name it (--source-snapshot,
18+
* --target-snapshot), so a larger read sees one instant throughout;
19+
* otherwise one a second connection exports and keeps open.
20+
*/
21+
public function take(Capsule $capsule, string $side, array $config, object $params): void {
22+
$id = $side === 'source' ? ($params->sourceSnapshot ?? null) : ($params->targetSnapshot ?? null);
23+
if ($id === null) {
24+
$anchor = $side . '__snapshot';
25+
$capsule->addConnection($config, $anchor);
26+
PostgresSessionOptions::apply($capsule->getConnection($anchor));
27+
$id = PostgresSnapshot::export($capsule->getConnection($anchor), $side);
28+
}
29+
if ($id !== null && PostgresSnapshot::join($capsule->getConnection($side), $id, $side)) {
30+
$this->ids[$side] = $id;
31+
}
32+
}
33+
34+
/** A connection made again for `$side` reads as of the same instant as the last. */
35+
public function rejoin(Connection $connection, string $side): void {
36+
if (isset($this->ids[$side]) && !PostgresSnapshot::join($connection, $this->ids[$side], $side)) {
37+
unset($this->ids[$side]);
38+
}
39+
}
40+
41+
/** Whether `$side` is read as of one snapshot. */
42+
public function holds(string $side): bool {
43+
return isset($this->ids[$side]);
44+
}
45+
}

‎src/DB/Support/ServerSettings.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
<?php namespace DBDiff\DB\Support;
22

3+
use DBDiff\DB\Adapters\DBAdapterInterface;
34
use DBDiff\Exceptions\DBException;
45

56
final class ServerSettings
@@ -17,4 +18,22 @@ public static function requireFor(mixed $server, string $name): void
1718
);
1819
}
1920
}
21+
22+
/**
23+
* The connection config for one side of the input: an SQLite file's path
24+
* is the db itself; a server's settings, with the sslmode --supabase or
25+
* --sslmode supply where its own URL names none.
26+
*/
27+
public static function connectionConfig(DBAdapterInterface $adapter, string $driver, object $params, array $input): array
28+
{
29+
if ($driver === 'sqlite') {
30+
return $adapter->buildConnectionConfig([], $input['db']);
31+
}
32+
$server = $params->{$input['server']};
33+
self::requireFor($server, $input['server']);
34+
if (isset($params->sslmode) && empty($server['sslmode'])) {
35+
$server['sslmode'] = $params->sslmode;
36+
}
37+
return $adapter->buildConnectionConfig($server, $input['db']);
38+
}
2039
}

‎src/Params/CLIGetter.php‎

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -26,32 +26,19 @@ public function getParams() {
2626
if ($getopt->get('--server2')) {
2727
$params->server2 = $this->parseServer($getopt->get('--server2'));
2828
}
29-
if ($getopt->get('--format')) {
30-
$params->format = $getopt->get('--format');
31-
}
32-
if ($getopt->get('--template')) {
33-
$params->template = $getopt->get('--template');
34-
}
35-
if ($getopt->get('--type')) {
36-
$params->type = $getopt->get('--type');
37-
}
38-
if ($getopt->get('--include')) {
39-
$params->include = $getopt->get('--include');
40-
}
41-
if ($getopt->get('--nocomments')) {
42-
$params->nocomments = $getopt->get('--nocomments');
29+
// Options taken as given.
30+
foreach (['format', 'template', 'type', 'include', 'nocomments'] as $option) {
31+
if ($getopt->get("--$option")) {
32+
$params->$option = $getopt->get("--$option");
33+
}
4334
}
4435
if ($getopt->get('--units')) {
4536
$params->units = (bool) $getopt->get('--units');
4637
}
47-
if ($getopt->get('--config')) {
48-
$params->config = $getopt->get('--config');
49-
}
50-
if ($getopt->get('--output')) {
51-
$params->output = $getopt->get('--output');
52-
}
53-
if ($getopt->get('--debug')) {
54-
$params->debug = $getopt->get('--debug');
38+
foreach (['config', 'output', 'debug'] as $option) {
39+
if ($getopt->get("--$option")) {
40+
$params->$option = $getopt->get("--$option");
41+
}
5542
}
5643
if ($getopt->get('--driver')) {
5744
$params->driver = strtolower($getopt->get('--driver'));

‎tests/SnapshotPostgresTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public function testATableMadeWhileTheDiffReadsIsNotSeen(): void
3838
$source = $this->db('s', 'CREATE TABLE before_connect (id int);');
3939
$target = $this->db('t', '');
4040
$manager = $this->manager($source, $target);
41-
$this->assertTrue($manager->readsSnapshot('source'));
41+
$this->assertTrue($manager->snapshots->holds('source'));
4242

4343
$this->connect($source)->exec('CREATE TABLE after_connect (id int);');
4444

@@ -50,7 +50,7 @@ public function testWithoutTheSnapshotItIs(): void
5050
$source = $this->db('ns', 'CREATE TABLE before_connect (id int);');
5151
$target = $this->db('nt', '');
5252
$manager = $this->manager($source, $target, false);
53-
$this->assertFalse($manager->readsSnapshot('source'));
53+
$this->assertFalse($manager->snapshots->holds('source'));
5454

5555
$this->connect($source)->exec('CREATE TABLE after_connect (id int);');
5656

@@ -66,7 +66,7 @@ public function testTheConnectionForAnotherSchemaReadsAsOfTheSameInstant(): void
6666
$this->connect($source)->exec('CREATE TABLE app.added (id int);');
6767
$manager->useSchema('app');
6868

69-
$this->assertTrue($manager->readsSnapshot('source'));
69+
$this->assertTrue($manager->snapshots->holds('source'));
7070
$this->assertSame(['kept'], $manager->getTables('source'));
7171
}
7272

@@ -102,7 +102,7 @@ public function testASnapshotAnotherSessionHoldsIsReadAsOf(): void
102102

103103
$manager = $this->manager($source, $target, true, $id);
104104

105-
$this->assertTrue($manager->readsSnapshot('source'));
105+
$this->assertTrue($manager->snapshots->holds('source'));
106106
$this->assertSame(['exported_before'], $manager->getTables('source'));
107107
$holder->exec('ROLLBACK');
108108
}

‎tests/Unit/MySQLColumnCharsetTest.php‎

Lines changed: 4 additions & 4 deletions

0 commit comments

Comments
 (0)