From 2c7962af2148583168fe079eefac91e956f6194f Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 15 Jul 2026 17:54:25 +1000 Subject: [PATCH 1/5] Add table prefix support to TableIdentifier with configurable factory Adds optional prefix and separator parameters to TableIdentifier. When a prefix is set, getTable() and getTableAndSchema() return the prefixed table name (prefix + separator + table), so the prefix flows through all SQL generation unchanged. The separator defaults to '_' and the original name remains available via getUnprefixedTable(). Adds the callable Sql\TableIdentifierFactory, which produces identifiers carrying a preconfigured prefix/separator, and the container factory Container\TableIdentifierFactoryFactory, which reads both from the 'config' service so the prefix can be set once globally (e.g. to create backup_* tables during a migration). Registered in ConfigProvider. Closes php-db/phpdb#147 --- src/ConfigProvider.php | 3 +- .../TableIdentifierFactoryFactory.php | 43 +++++++ src/Sql/TableIdentifier.php | 52 ++++++++- src/Sql/TableIdentifierFactory.php | 57 ++++++++++ test/unit/ConfigProviderTest.php | 4 +- .../TableIdentifierFactoryFactoryTest.php | 84 ++++++++++++++ test/unit/Sql/TableIdentifierFactoryTest.php | 105 ++++++++++++++++++ test/unit/Sql/TableIdentifierTest.php | 85 ++++++++++++++ 8 files changed, 428 insertions(+), 5 deletions(-) create mode 100644 src/Container/TableIdentifierFactoryFactory.php create mode 100644 src/Sql/TableIdentifierFactory.php create mode 100644 test/unit/Container/TableIdentifierFactoryFactoryTest.php create mode 100644 test/unit/Sql/TableIdentifierFactoryTest.php diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php index eb1758d9..4fea5d94 100644 --- a/src/ConfigProvider.php +++ b/src/ConfigProvider.php @@ -25,7 +25,8 @@ public function getDependencies(): array Adapter\AdapterInterface::class => Adapter\Adapter::class, ], 'factories' => [ - Adapter\Adapter::class => Container\AdapterInterfaceFactory::class, + Adapter\Adapter::class => Container\AdapterInterfaceFactory::class, + Sql\TableIdentifierFactory::class => Container\TableIdentifierFactoryFactory::class, ], ]; } diff --git a/src/Container/TableIdentifierFactoryFactory.php b/src/Container/TableIdentifierFactoryFactory.php new file mode 100644 index 00000000..c48f0989 --- /dev/null +++ b/src/Container/TableIdentifierFactoryFactory.php @@ -0,0 +1,43 @@ + + * return [ + * TableIdentifierFactory::class => [ + * 'prefix' => 'backup', + * 'separator' => '_', + * ], + * ]; + * + * + * When no configuration is present the factory is created without a prefix. + * The separator defaults to '_' when not configured. + */ +final class TableIdentifierFactoryFactory +{ + public function __invoke(ContainerInterface $container): TableIdentifierFactory + { + /** @var array $config */ + $config = $container->has('config') ? $container->get('config') ?? [] : []; + + /** @var array{prefix?: string|null, separator?: string} $factoryConfig */ + $factoryConfig = $config[TableIdentifierFactory::class] ?? []; + + return new TableIdentifierFactory( + $factoryConfig['prefix'] ?? null, + $factoryConfig['separator'] ?? '_', + ); + } +} diff --git a/src/Sql/TableIdentifier.php b/src/Sql/TableIdentifier.php index eb5daee8..ec3f815e 100644 --- a/src/Sql/TableIdentifier.php +++ b/src/Sql/TableIdentifier.php @@ -10,8 +10,16 @@ class TableIdentifier protected ?string $schema = null; - public function __construct(string $table, ?string $schema = null) - { + protected ?string $prefix = null; + + protected string $separator = '_'; + + public function __construct( + string $table, + ?string $schema = null, + ?string $prefix = null, + string $separator = '_', + ) { if ('' === $table) { throw new Exception\InvalidArgumentException( '$table must be a valid table name, empty string given' @@ -29,13 +37,51 @@ public function __construct(string $table, ?string $schema = null) $this->schema = $schema; } + + if ($prefix !== null) { + if ('' === $prefix) { + throw new Exception\InvalidArgumentException( + '$prefix must be a valid table prefix or null, empty string given' + ); + } + + $this->prefix = $prefix; + } + + $this->separator = $separator; } + /** + * Returns the table name with the prefix and separator applied, when a + * prefix is set. + */ public function getTable(): string + { + if ($this->prefix === null) { + return $this->table; + } + + return $this->prefix . $this->separator . $this->table; + } + + /** + * Returns the table name as given, without the prefix applied. + */ + public function getUnprefixedTable(): string { return $this->table; } + public function getPrefix(): ?string + { + return $this->prefix; + } + + public function getSeparator(): string + { + return $this->separator; + } + public function getSchema(): ?string { return $this->schema; @@ -44,6 +90,6 @@ public function getSchema(): ?string /** @return array{0: string, 1: null|string} */ public function getTableAndSchema(): array { - return [$this->table, $this->schema]; + return [$this->getTable(), $this->schema]; } } diff --git a/src/Sql/TableIdentifierFactory.php b/src/Sql/TableIdentifierFactory.php new file mode 100644 index 00000000..28807092 --- /dev/null +++ b/src/Sql/TableIdentifierFactory.php @@ -0,0 +1,57 @@ +prefix = $prefix; + } + + public function getPrefix(): ?string + { + return $this->prefix; + } + + public function getSeparator(): string + { + return $this->separator; + } + + /** + * Creates a TableIdentifier carrying the configured prefix and separator. + * + * A prefix or separator passed at call time takes precedence over the + * configured one. + */ + public function __invoke( + string $table, + ?string $schema = null, + ?string $prefix = null, + ?string $separator = null, + ): TableIdentifier { + return new TableIdentifier($table, $schema, $prefix ?? $this->prefix, $separator ?? $this->separator); + } +} diff --git a/test/unit/ConfigProviderTest.php b/test/unit/ConfigProviderTest.php index 9fc3a0e2..ab643db7 100644 --- a/test/unit/ConfigProviderTest.php +++ b/test/unit/ConfigProviderTest.php @@ -7,6 +7,7 @@ use PhpDb\Adapter; use PhpDb\ConfigProvider; use PhpDb\Container; +use PhpDb\Sql; use PHPUnit\Framework\TestCase; class ConfigProviderTest extends TestCase @@ -29,7 +30,8 @@ class ConfigProviderTest extends TestCase Adapter\AdapterInterface::class => Adapter\Adapter::class, ], 'factories' => [ - Adapter\Adapter::class => Container\AdapterInterfaceFactory::class, + Adapter\Adapter::class => Container\AdapterInterfaceFactory::class, + Sql\TableIdentifierFactory::class => Container\TableIdentifierFactoryFactory::class, ], ], ]; diff --git a/test/unit/Container/TableIdentifierFactoryFactoryTest.php b/test/unit/Container/TableIdentifierFactoryFactoryTest.php new file mode 100644 index 00000000..791884fd --- /dev/null +++ b/test/unit/Container/TableIdentifierFactoryFactoryTest.php @@ -0,0 +1,84 @@ +getPrefix()); + } + + public function testInvokeCreatesFactoryWithoutPrefixWhenConfigIsEmpty(): void + { + $container = new ServiceManager(); + $container->setService('config', []); + + $factory = new TableIdentifierFactoryFactory(); + $result = $factory($container); + + self::assertNull($result->getPrefix()); + } + + public function testInvokeCreatesFactoryWithConfiguredPrefix(): void + { + $container = new ServiceManager(); + $container->setService('config', [ + TableIdentifierFactory::class => [ + 'prefix' => 'backup', + ], + ]); + + $factory = new TableIdentifierFactoryFactory(); + $result = $factory($container); + + self::assertSame('backup', $result->getPrefix()); + self::assertSame('backup_users', $result('users')->getTable()); + } + + public function testInvokeCreatesFactoryWithConfiguredSeparator(): void + { + $container = new ServiceManager(); + $container->setService('config', [ + TableIdentifierFactory::class => [ + 'prefix' => 'backup', + 'separator' => '__', + ], + ]); + + $factory = new TableIdentifierFactoryFactory(); + $result = $factory($container); + + self::assertSame('__', $result->getSeparator()); + self::assertSame('backup__users', $result('users')->getTable()); + } + + public function testInvokeCreatesFactoryWithoutPrefixWhenPrefixKeyIsAbsent(): void + { + $container = new ServiceManager(); + $container->setService('config', [ + TableIdentifierFactory::class => [], + ]); + + $factory = new TableIdentifierFactoryFactory(); + $result = $factory($container); + + self::assertNull($result->getPrefix()); + } +} diff --git a/test/unit/Sql/TableIdentifierFactoryTest.php b/test/unit/Sql/TableIdentifierFactoryTest.php new file mode 100644 index 00000000..d6a3a2ac --- /dev/null +++ b/test/unit/Sql/TableIdentifierFactoryTest.php @@ -0,0 +1,105 @@ +getPrefix()); + } + + public function testGetPrefix(): void + { + $factory = new TableIdentifierFactory('backup'); + + self::assertSame('backup', $factory->getPrefix()); + } + + public function testGetDefaultSeparator(): void + { + $factory = new TableIdentifierFactory('backup'); + + self::assertSame('_', $factory->getSeparator()); + } + + public function testGetSeparator(): void + { + $factory = new TableIdentifierFactory('backup', '__'); + + self::assertSame('__', $factory->getSeparator()); + } + + public function testRejectsEmptyStringPrefix(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('$prefix must be a valid table prefix or null, empty string given'); + new TableIdentifierFactory(''); + } + + public function testCreatesIdentifierWithConfiguredPrefix(): void + { + $factory = new TableIdentifierFactory('backup'); + $tableIdentifier = $factory('users'); + + self::assertSame('backup', $tableIdentifier->getPrefix()); + self::assertSame('backup_users', $tableIdentifier->getTable()); + self::assertNull($tableIdentifier->getSchema()); + } + + public function testCreatesIdentifierWithSchema(): void + { + $factory = new TableIdentifierFactory('backup'); + $tableIdentifier = $factory('users', 'public'); + + self::assertSame(['backup_users', 'public'], $tableIdentifier->getTableAndSchema()); + } + + public function testCreatesIdentifierWithConfiguredSeparator(): void + { + $factory = new TableIdentifierFactory('backup', '__'); + $tableIdentifier = $factory('users'); + + self::assertSame('__', $tableIdentifier->getSeparator()); + self::assertSame('backup__users', $tableIdentifier->getTable()); + } + + public function testCreatesIdentifierWithoutPrefixWhenNoneConfigured(): void + { + $factory = new TableIdentifierFactory(); + $tableIdentifier = $factory('users', 'public'); + + self::assertNull($tableIdentifier->getPrefix()); + self::assertSame('users', $tableIdentifier->getTable()); + } + + public function testCallTimePrefixOverridesConfiguredPrefix(): void + { + $factory = new TableIdentifierFactory('backup'); + $tableIdentifier = $factory('users', null, 'archive'); + + self::assertSame('archive', $tableIdentifier->getPrefix()); + self::assertSame('archive_users', $tableIdentifier->getTable()); + } + + public function testCallTimeSeparatorOverridesConfiguredSeparator(): void + { + $factory = new TableIdentifierFactory('backup', '__'); + $tableIdentifier = $factory('users', null, null, '_'); + + self::assertSame('_', $tableIdentifier->getSeparator()); + self::assertSame('backup_users', $tableIdentifier->getTable()); + } +} diff --git a/test/unit/Sql/TableIdentifierTest.php b/test/unit/Sql/TableIdentifierTest.php index 2e6c717e..410b9a6c 100644 --- a/test/unit/Sql/TableIdentifierTest.php +++ b/test/unit/Sql/TableIdentifierTest.php @@ -63,6 +63,83 @@ public function testGetSchemaFromObjectStringCast(): void self::assertSame('castResult', $tableIdentifier->getSchema()); } + public function testGetDefaultPrefix(): void + { + $tableIdentifier = new TableIdentifier('foo'); + + self::assertNull($tableIdentifier->getPrefix()); + } + + public function testGetPrefix(): void + { + $tableIdentifier = new TableIdentifier('foo', null, 'backup'); + + self::assertSame('backup', $tableIdentifier->getPrefix()); + } + + public function testGetDefaultSeparator(): void + { + $tableIdentifier = new TableIdentifier('foo'); + + self::assertSame('_', $tableIdentifier->getSeparator()); + } + + public function testGetSeparator(): void + { + $tableIdentifier = new TableIdentifier('foo', null, 'backup', '__'); + + self::assertSame('__', $tableIdentifier->getSeparator()); + } + + public function testGetTableAppliesPrefixWithDefaultSeparator(): void + { + $tableIdentifier = new TableIdentifier('foo', null, 'backup'); + + self::assertSame('backup_foo', $tableIdentifier->getTable()); + } + + public function testGetTableAppliesPrefixWithCustomSeparator(): void + { + $tableIdentifier = new TableIdentifier('foo', null, 'backup', '__'); + + self::assertSame('backup__foo', $tableIdentifier->getTable()); + } + + public function testGetTableAppliesPrefixWithEmptySeparator(): void + { + $tableIdentifier = new TableIdentifier('foo', null, 'backup', ''); + + self::assertSame('backupfoo', $tableIdentifier->getTable()); + } + + public function testGetTableIgnoresSeparatorWithoutPrefix(): void + { + $tableIdentifier = new TableIdentifier('foo', null, null, '__'); + + self::assertSame('foo', $tableIdentifier->getTable()); + } + + public function testGetUnprefixedTableReturnsTableAsGiven(): void + { + $tableIdentifier = new TableIdentifier('foo', null, 'backup'); + + self::assertSame('foo', $tableIdentifier->getUnprefixedTable()); + } + + public function testGetTableAndSchemaAppliesPrefix(): void + { + $tableIdentifier = new TableIdentifier('foo', 'bar', 'backup'); + + self::assertSame(['backup_foo', 'bar'], $tableIdentifier->getTableAndSchema()); + } + + public function testGetTableAndSchemaWithoutPrefix(): void + { + $tableIdentifier = new TableIdentifier('foo', 'bar'); + + self::assertSame(['foo', 'bar'], $tableIdentifier->getTableAndSchema()); + } + #[DataProvider('invalidTableProvider')] public function testRejectsInvalidTable(mixed $invalidTable): void { @@ -79,6 +156,14 @@ public function testRejectsInvalidSchema(mixed $invalidSchema): void new TableIdentifier('foo', $invalidSchema); } + #[DataProvider('invalidSchemaProvider')] + public function testRejectsInvalidPrefix(mixed $invalidPrefix): void + { + $this->expectException($invalidPrefix === '' ? InvalidArgumentException::class : TypeError::class); + /** @psalm-suppress MixedArgument */ + new TableIdentifier('foo', 'bar', $invalidPrefix); + } + /** * Data provider * From d1f9228499ad73826d9cb4328893dd3001320991 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Thu, 16 Jul 2026 10:09:32 +1000 Subject: [PATCH 2/5] - Improvements to testing - Updated DDL documentation --- docs/book/sql-ddl/alter-drop.md | 55 +++++++++++++++++++ test/unit/ConfigProviderTest.php | 4 ++ .../TableIdentifierFactoryFactoryTest.php | 15 ++++- test/unit/Sql/TableIdentifierFactoryTest.php | 8 +-- test/unit/Sql/TableIdentifierTest.php | 18 +++--- 5 files changed, 86 insertions(+), 14 deletions(-) diff --git a/docs/book/sql-ddl/alter-drop.md b/docs/book/sql-ddl/alter-drop.md index c0105350..3d2b9619 100644 --- a/docs/book/sql-ddl/alter-drop.md +++ b/docs/book/sql-ddl/alter-drop.md @@ -430,6 +430,61 @@ $fk = new ForeignKey( ); ``` +### Table Prefixes + +`TableIdentifier` accepts an optional prefix and separator. When a prefix is +set, `getTable()` and `getTableAndSchema()` return the prefixed table name; +`getUnprefixedTable()` returns the table name as given: + +```php title="Prefixed Table Identifiers" +use PhpDb\Sql\TableIdentifier; + +$identifier = new TableIdentifier('users', null, 'backup'); +$identifier->getTable(); // 'backup_users' +$identifier->getUnprefixedTable(); // 'users' + +// Custom separator +$identifier = new TableIdentifier('users', null, 'backup', '__'); +$identifier->getTable(); // 'backup__users' +``` + +The prefix must be a non-empty string or `null`; the separator defaults to +`'_'`. + +### The TableIdentifierFactory + +`PhpDb\Sql\TableIdentifierFactory` is a callable factory that creates +`TableIdentifier` instances sharing a preconfigured prefix and separator: + +```php title="Creating Identifiers via the Factory" +use PhpDb\Sql\TableIdentifierFactory; + +$factory = new TableIdentifierFactory('backup'); + +$factory('users')->getTable(); // 'backup_users' +$factory('orders', 'sales')->getTable(); // 'backup_orders' + +// A prefix or separator passed at call time overrides the configured one +$factory('users', null, 'archive')->getTable(); // 'archive_users' +``` + +`PhpDb\ConfigProvider` registers the factory as a container service. Configure +the prefix and separator through the application config: + +```php title="Configuring the Factory Service" +use PhpDb\Sql\TableIdentifierFactory; + +return [ + TableIdentifierFactory::class => [ + 'prefix' => 'backup', + 'separator' => '_', + ], +]; +``` + +When no configuration is present the service is created without a prefix, and +the separator defaults to `'_'`. + ## Nullable and Default Values ### Setting Nullable diff --git a/test/unit/ConfigProviderTest.php b/test/unit/ConfigProviderTest.php index ab643db7..085f05ee 100644 --- a/test/unit/ConfigProviderTest.php +++ b/test/unit/ConfigProviderTest.php @@ -8,8 +8,12 @@ use PhpDb\ConfigProvider; use PhpDb\Container; use PhpDb\Sql; +use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +#[CoversClass(ConfigProvider::class)] +#[Group('unit')] class ConfigProviderTest extends TestCase { /** diff --git a/test/unit/Container/TableIdentifierFactoryFactoryTest.php b/test/unit/Container/TableIdentifierFactoryFactoryTest.php index 791884fd..4a0d429a 100644 --- a/test/unit/Container/TableIdentifierFactoryFactoryTest.php +++ b/test/unit/Container/TableIdentifierFactoryFactoryTest.php @@ -10,6 +10,7 @@ use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; +use Psr\Container\ContainerInterface; #[Group('unit')] #[CoversMethod(TableIdentifierFactoryFactory::class, '__invoke')] @@ -49,7 +50,6 @@ public function testInvokeCreatesFactoryWithConfiguredPrefix(): void $result = $factory($container); self::assertSame('backup', $result->getPrefix()); - self::assertSame('backup_users', $result('users')->getTable()); } public function testInvokeCreatesFactoryWithConfiguredSeparator(): void @@ -66,7 +66,18 @@ public function testInvokeCreatesFactoryWithConfiguredSeparator(): void $result = $factory($container); self::assertSame('__', $result->getSeparator()); - self::assertSame('backup__users', $result('users')->getTable()); + } + + public function testInvokeCreatesFactoryWithoutPrefixWhenConfigServiceIsNull(): void + { + $container = $this->createMock(ContainerInterface::class); + $container->method('has')->with('config')->willReturn(true); + $container->method('get')->with('config')->willReturn(null); + + $factory = new TableIdentifierFactoryFactory(); + $result = $factory($container); + + self::assertNull($result->getPrefix()); } public function testInvokeCreatesFactoryWithoutPrefixWhenPrefixKeyIsAbsent(): void diff --git a/test/unit/Sql/TableIdentifierFactoryTest.php b/test/unit/Sql/TableIdentifierFactoryTest.php index d6a3a2ac..c407dd5c 100644 --- a/test/unit/Sql/TableIdentifierFactoryTest.php +++ b/test/unit/Sql/TableIdentifierFactoryTest.php @@ -14,28 +14,28 @@ #[CoversClass(TableIdentifierFactory::class)] final class TableIdentifierFactoryTest extends TestCase { - public function testGetDefaultPrefix(): void + public function testPrefixIsNullByDefault(): void { $factory = new TableIdentifierFactory(); self::assertNull($factory->getPrefix()); } - public function testGetPrefix(): void + public function testReturnsConfiguredPrefix(): void { $factory = new TableIdentifierFactory('backup'); self::assertSame('backup', $factory->getPrefix()); } - public function testGetDefaultSeparator(): void + public function testSeparatorDefaultsToUnderscore(): void { $factory = new TableIdentifierFactory('backup'); self::assertSame('_', $factory->getSeparator()); } - public function testGetSeparator(): void + public function testReturnsConfiguredSeparator(): void { $factory = new TableIdentifierFactory('backup', '__'); diff --git a/test/unit/Sql/TableIdentifierTest.php b/test/unit/Sql/TableIdentifierTest.php index 410b9a6c..7ca707b8 100644 --- a/test/unit/Sql/TableIdentifierTest.php +++ b/test/unit/Sql/TableIdentifierTest.php @@ -9,6 +9,7 @@ use PhpDbTest\TestAsset\ObjectToString; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\DataProvider; +use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; use stdClass; use TypeError; @@ -19,6 +20,7 @@ * Tests for {@see TableIdentifier} */ #[CoversClass(TableIdentifier::class)] +#[Group('unit')] class TableIdentifierTest extends TestCase { public function testGetTable(): void @@ -148,7 +150,7 @@ public function testRejectsInvalidTable(mixed $invalidTable): void new TableIdentifier($invalidTable); } - #[DataProvider('invalidSchemaProvider')] + #[DataProvider('invalidNameArgumentProvider')] public function testRejectsInvalidSchema(mixed $invalidSchema): void { $this->expectException($invalidSchema === '' ? InvalidArgumentException::class : TypeError::class); @@ -156,7 +158,7 @@ public function testRejectsInvalidSchema(mixed $invalidSchema): void new TableIdentifier('foo', $invalidSchema); } - #[DataProvider('invalidSchemaProvider')] + #[DataProvider('invalidNameArgumentProvider')] public function testRejectsInvalidPrefix(mixed $invalidPrefix): void { $this->expectException($invalidPrefix === '' ? InvalidArgumentException::class : TypeError::class); @@ -172,8 +174,8 @@ public function testRejectsInvalidPrefix(mixed $invalidPrefix): void public static function invalidTableProvider(): array { return array_merge( - [[null]], - self::invalidSchemaProvider() + ['null' => [null]], + self::invalidNameArgumentProvider() ); } @@ -182,12 +184,12 @@ public static function invalidTableProvider(): array * * @return array[] */ - public static function invalidSchemaProvider(): array + public static function invalidNameArgumentProvider(): array { return [ - [''], - [new stdClass()], - [[]], + 'empty string' => [''], + 'object' => [new stdClass()], + 'array' => [[]], ]; } } From 8f3207a47d1b2f410aac1728e40b70fe87e5ecc2 Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 22 Jul 2026 16:47:37 +1000 Subject: [PATCH 3/5] Added separator CONST to TableIdentifier Used separator CONST for Factory --- src/Sql/TableIdentifier.php | 4 +++- src/Sql/TableIdentifierFactory.php | 10 +++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/Sql/TableIdentifier.php b/src/Sql/TableIdentifier.php index ec3f815e..07a1530d 100644 --- a/src/Sql/TableIdentifier.php +++ b/src/Sql/TableIdentifier.php @@ -6,13 +6,15 @@ class TableIdentifier { + public const SEPARATOR = '_'; + protected string $table; protected ?string $schema = null; protected ?string $prefix = null; - protected string $separator = '_'; + protected string $separator = self::SEPARATOR; public function __construct( string $table, diff --git a/src/Sql/TableIdentifierFactory.php b/src/Sql/TableIdentifierFactory.php index 28807092..18a0b99d 100644 --- a/src/Sql/TableIdentifierFactory.php +++ b/src/Sql/TableIdentifierFactory.php @@ -13,21 +13,17 @@ * factory share the same prefix — convenient for creating backup_* tables * during a migration. */ -final class TableIdentifierFactory +final readonly class TableIdentifierFactory { - private readonly ?string $prefix; - public function __construct( - ?string $prefix = null, - private readonly string $separator = '_', + private ?string $prefix = null, + private ?string $separator = TableIdentifier::SEPARATOR, ) { if ('' === $prefix) { throw new Exception\InvalidArgumentException( '$prefix must be a valid table prefix or null, empty string given' ); } - - $this->prefix = $prefix; } public function getPrefix(): ?string From 6c8b87cc8ae3fccc36412723ed69cd36c197b54e Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Wed, 22 Jul 2026 16:58:26 +1000 Subject: [PATCH 4/5] Updated testing to ensure a non-empty string for separator Updated documentation --- docs/book/sql-ddl/alter-drop.md | 11 ++- src/Sql/TableIdentifier.php | 9 +++ src/Sql/TableIdentifierFactory.php | 16 +++- .../TableIdentifierFactoryFactoryTest.php | 48 ++++++++++++ test/unit/Sql/TableIdentifierFactoryTest.php | 74 +++++++++++++++++++ test/unit/Sql/TableIdentifierTest.php | 22 ++++-- 6 files changed, 169 insertions(+), 11 deletions(-) diff --git a/docs/book/sql-ddl/alter-drop.md b/docs/book/sql-ddl/alter-drop.md index 3d2b9619..bd2bd895 100644 --- a/docs/book/sql-ddl/alter-drop.md +++ b/docs/book/sql-ddl/alter-drop.md @@ -448,8 +448,9 @@ $identifier = new TableIdentifier('users', null, 'backup', '__'); $identifier->getTable(); // 'backup__users' ``` -The prefix must be a non-empty string or `null`; the separator defaults to -`'_'`. +The prefix must be a non-empty string or `null`. The separator defaults to +`'_'` and must also be a non-empty string; passing `''` for either throws +`PhpDb\Sql\Exception\InvalidArgumentException`. ### The TableIdentifierFactory @@ -468,6 +469,12 @@ $factory('orders', 'sales')->getTable(); // 'backup_orders' $factory('users', null, 'archive')->getTable(); // 'archive_users' ``` +The configured separator is only applied when a prefix is passed, so a factory +created with a separator but no prefix produces unprefixed identifiers. Passing +`null` as the separator falls back to the default `'_'`; passing `''` — at +construction or at call time — throws +`PhpDb\Sql\Exception\InvalidArgumentException`. + `PhpDb\ConfigProvider` registers the factory as a container service. Configure the prefix and separator through the application config: diff --git a/src/Sql/TableIdentifier.php b/src/Sql/TableIdentifier.php index 07a1530d..68bf8248 100644 --- a/src/Sql/TableIdentifier.php +++ b/src/Sql/TableIdentifier.php @@ -16,6 +16,9 @@ class TableIdentifier protected string $separator = self::SEPARATOR; + /** + * @throws Exception\InvalidArgumentException If $table, $schema, $prefix or $separator is an empty string. + */ public function __construct( string $table, ?string $schema = null, @@ -50,6 +53,12 @@ public function __construct( $this->prefix = $prefix; } + if ('' === $separator) { + throw new Exception\InvalidArgumentException( + '$separator must be a valid table separator, empty string given' + ); + } + $this->separator = $separator; } diff --git a/src/Sql/TableIdentifierFactory.php b/src/Sql/TableIdentifierFactory.php index 18a0b99d..316803e4 100644 --- a/src/Sql/TableIdentifierFactory.php +++ b/src/Sql/TableIdentifierFactory.php @@ -15,6 +15,10 @@ */ final readonly class TableIdentifierFactory { + /** + * @param null|string $separator Null falls back to {@see TableIdentifier::SEPARATOR}. + * @throws Exception\InvalidArgumentException If $prefix or $separator is an empty string. + */ public function __construct( private ?string $prefix = null, private ?string $separator = TableIdentifier::SEPARATOR, @@ -24,6 +28,12 @@ public function __construct( '$prefix must be a valid table prefix or null, empty string given' ); } + + if ('' === $separator) { + throw new Exception\InvalidArgumentException( + '$separator must be a valid table separator, empty string given' + ); + } } public function getPrefix(): ?string @@ -33,7 +43,7 @@ public function getPrefix(): ?string public function getSeparator(): string { - return $this->separator; + return $this->separator ?? TableIdentifier::SEPARATOR; } /** @@ -41,6 +51,8 @@ public function getSeparator(): string * * A prefix or separator passed at call time takes precedence over the * configured one. + * + * @throws Exception\InvalidArgumentException If $prefix or $separator is an empty string. */ public function __invoke( string $table, @@ -48,6 +60,6 @@ public function __invoke( ?string $prefix = null, ?string $separator = null, ): TableIdentifier { - return new TableIdentifier($table, $schema, $prefix ?? $this->prefix, $separator ?? $this->separator); + return new TableIdentifier($table, $schema, $prefix ?? $this->prefix, $separator ?? $this->getSeparator()); } } diff --git a/test/unit/Container/TableIdentifierFactoryFactoryTest.php b/test/unit/Container/TableIdentifierFactoryFactoryTest.php index 4a0d429a..9f37ca6f 100644 --- a/test/unit/Container/TableIdentifierFactoryFactoryTest.php +++ b/test/unit/Container/TableIdentifierFactoryFactoryTest.php @@ -6,6 +6,7 @@ use Laminas\ServiceManager\ServiceManager; use PhpDb\Container\TableIdentifierFactoryFactory; +use PhpDb\Sql\Exception\InvalidArgumentException; use PhpDb\Sql\TableIdentifierFactory; use PHPUnit\Framework\Attributes\CoversMethod; use PHPUnit\Framework\Attributes\Group; @@ -80,6 +81,53 @@ public function testInvokeCreatesFactoryWithoutPrefixWhenConfigServiceIsNull(): self::assertNull($result->getPrefix()); } + public function testInvokeCreatesFactoryWithConfiguredSeparatorWithoutPrefix(): void + { + $container = new ServiceManager(); + $container->setService('config', [ + TableIdentifierFactory::class => [ + 'separator' => '__', + ], + ]); + + $factory = new TableIdentifierFactoryFactory(); + $result = $factory($container); + + self::assertNull($result->getPrefix()); + self::assertSame('__', $result->getSeparator()); + } + + public function testInvokeUsesDefaultSeparatorWhenSeparatorKeyIsAbsent(): void + { + $container = new ServiceManager(); + $container->setService('config', [ + TableIdentifierFactory::class => [ + 'prefix' => 'backup', + ], + ]); + + $factory = new TableIdentifierFactoryFactory(); + $result = $factory($container); + + self::assertSame('_', $result->getSeparator()); + } + + public function testInvokeRejectsEmptyStringSeparatorFromConfig(): void + { + $container = new ServiceManager(); + $container->setService('config', [ + TableIdentifierFactory::class => [ + 'separator' => '', + ], + ]); + + $factory = new TableIdentifierFactoryFactory(); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('$separator must be a valid table separator, empty string given'); + $factory($container); + } + public function testInvokeCreatesFactoryWithoutPrefixWhenPrefixKeyIsAbsent(): void { $container = new ServiceManager(); diff --git a/test/unit/Sql/TableIdentifierFactoryTest.php b/test/unit/Sql/TableIdentifierFactoryTest.php index c407dd5c..28846ed9 100644 --- a/test/unit/Sql/TableIdentifierFactoryTest.php +++ b/test/unit/Sql/TableIdentifierFactoryTest.php @@ -35,6 +35,20 @@ public function testSeparatorDefaultsToUnderscore(): void self::assertSame('_', $factory->getSeparator()); } + public function testSeparatorDefaultsToUnderscoreWhenNoPrefixConfigured(): void + { + $factory = new TableIdentifierFactory(); + + self::assertSame('_', $factory->getSeparator()); + } + + public function testSeparatorFallsBackToDefaultWhenPassedAsNull(): void + { + $factory = new TableIdentifierFactory('backup', null); + + self::assertSame('_', $factory->getSeparator()); + } + public function testReturnsConfiguredSeparator(): void { $factory = new TableIdentifierFactory('backup', '__'); @@ -49,6 +63,20 @@ public function testRejectsEmptyStringPrefix(): void new TableIdentifierFactory(''); } + public function testRejectsEmptyStringSeparator(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('$separator must be a valid table separator, empty string given'); + new TableIdentifierFactory('backup', ''); + } + + public function testRejectsEmptyStringSeparatorWithoutPrefix(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('$separator must be a valid table separator, empty string given'); + new TableIdentifierFactory(null, ''); + } + public function testCreatesIdentifierWithConfiguredPrefix(): void { $factory = new TableIdentifierFactory('backup'); @@ -102,4 +130,50 @@ public function testCallTimeSeparatorOverridesConfiguredSeparator(): void self::assertSame('_', $tableIdentifier->getSeparator()); self::assertSame('backup_users', $tableIdentifier->getTable()); } + + public function testCallTimePrefixIsAppliedWhenNonePreconfigured(): void + { + $factory = new TableIdentifierFactory(); + $tableIdentifier = $factory('users', null, 'archive'); + + self::assertSame('archive', $tableIdentifier->getPrefix()); + self::assertSame('archive_users', $tableIdentifier->getTable()); + } + + public function testCallTimeSeparatorIsAppliedWhenNonePreconfigured(): void + { + $factory = new TableIdentifierFactory(); + $tableIdentifier = $factory('users', null, 'archive', '__'); + + self::assertSame('__', $tableIdentifier->getSeparator()); + self::assertSame('archive__users', $tableIdentifier->getTable()); + } + + public function testConfiguredSeparatorIsCarriedButUnusedWhenNoPrefixApplies(): void + { + $factory = new TableIdentifierFactory(null, '__'); + $tableIdentifier = $factory('users'); + + self::assertSame('__', $tableIdentifier->getSeparator()); + self::assertNull($tableIdentifier->getPrefix()); + self::assertSame('users', $tableIdentifier->getTable()); + } + + public function testRejectsEmptyStringCallTimePrefix(): void + { + $factory = new TableIdentifierFactory('backup'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('$prefix must be a valid table prefix or null, empty string given'); + $factory('users', null, ''); + } + + public function testRejectsEmptyStringCallTimeSeparator(): void + { + $factory = new TableIdentifierFactory('backup'); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('$separator must be a valid table separator, empty string given'); + $factory('users', null, null, ''); + } } diff --git a/test/unit/Sql/TableIdentifierTest.php b/test/unit/Sql/TableIdentifierTest.php index 7ca707b8..10f667f1 100644 --- a/test/unit/Sql/TableIdentifierTest.php +++ b/test/unit/Sql/TableIdentifierTest.php @@ -107,13 +107,6 @@ public function testGetTableAppliesPrefixWithCustomSeparator(): void self::assertSame('backup__foo', $tableIdentifier->getTable()); } - public function testGetTableAppliesPrefixWithEmptySeparator(): void - { - $tableIdentifier = new TableIdentifier('foo', null, 'backup', ''); - - self::assertSame('backupfoo', $tableIdentifier->getTable()); - } - public function testGetTableIgnoresSeparatorWithoutPrefix(): void { $tableIdentifier = new TableIdentifier('foo', null, null, '__'); @@ -166,6 +159,21 @@ public function testRejectsInvalidPrefix(mixed $invalidPrefix): void new TableIdentifier('foo', 'bar', $invalidPrefix); } + #[DataProvider('invalidNameArgumentProvider')] + public function testRejectsInvalidSeparator(mixed $invalidSeparator): void + { + $this->expectException($invalidSeparator === '' ? InvalidArgumentException::class : TypeError::class); + /** @psalm-suppress MixedArgument */ + new TableIdentifier('foo', 'bar', 'backup', $invalidSeparator); + } + + public function testRejectsEmptyStringSeparatorWithoutPrefix(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('$separator must be a valid table separator, empty string given'); + new TableIdentifier('foo', null, null, ''); + } + /** * Data provider * From 642f5bd120ede81af82e17ff3f11750594619e7f Mon Sep 17 00:00:00 2001 From: Simon Mundy Date: Thu, 23 Jul 2026 15:11:42 +1000 Subject: [PATCH 5/5] Fixes as per code review --- src/ConfigProvider.php | 2 +- .../TableIdentifierFactoryFactory.php | 3 +- src/Sql/TableIdentifier.php | 46 ++++++------------- 3 files changed, 16 insertions(+), 35 deletions(-) diff --git a/src/ConfigProvider.php b/src/ConfigProvider.php index 4fea5d94..78d69ac0 100644 --- a/src/ConfigProvider.php +++ b/src/ConfigProvider.php @@ -4,7 +4,7 @@ namespace PhpDb; -final class ConfigProvider +final readonly class ConfigProvider { public const NAMED_ADAPTER_KEY = 'adapters'; diff --git a/src/Container/TableIdentifierFactoryFactory.php b/src/Container/TableIdentifierFactoryFactory.php index c48f0989..6f918434 100644 --- a/src/Container/TableIdentifierFactoryFactory.php +++ b/src/Container/TableIdentifierFactoryFactory.php @@ -4,6 +4,7 @@ namespace PhpDb\Container; +use PhpDb\Sql\TableIdentifier; use PhpDb\Sql\TableIdentifierFactory; use Psr\Container\ContainerInterface; @@ -37,7 +38,7 @@ public function __invoke(ContainerInterface $container): TableIdentifierFactory return new TableIdentifierFactory( $factoryConfig['prefix'] ?? null, - $factoryConfig['separator'] ?? '_', + $factoryConfig['separator'] ?? TableIdentifier::SEPARATOR, ); } } diff --git a/src/Sql/TableIdentifier.php b/src/Sql/TableIdentifier.php index 68bf8248..76337e8d 100644 --- a/src/Sql/TableIdentifier.php +++ b/src/Sql/TableIdentifier.php @@ -4,26 +4,18 @@ namespace PhpDb\Sql; -class TableIdentifier +final readonly class TableIdentifier { public const SEPARATOR = '_'; - protected string $table; - - protected ?string $schema = null; - - protected ?string $prefix = null; - - protected string $separator = self::SEPARATOR; - /** * @throws Exception\InvalidArgumentException If $table, $schema, $prefix or $separator is an empty string. */ public function __construct( - string $table, - ?string $schema = null, - ?string $prefix = null, - string $separator = '_', + protected string $table, + protected ?string $schema = null, + protected ?string $prefix = null, + protected ?string $separator = self::SEPARATOR, ) { if ('' === $table) { throw new Exception\InvalidArgumentException( @@ -31,26 +23,16 @@ public function __construct( ); } - $this->table = $table; - - if ($schema !== null) { - if ('' === $schema) { - throw new Exception\InvalidArgumentException( - '$schema must be a valid schema name or null, empty string given' - ); - } - - $this->schema = $schema; + if ('' === $schema) { + throw new Exception\InvalidArgumentException( + '$schema must be a valid schema name or null, empty string given' + ); } - if ($prefix !== null) { - if ('' === $prefix) { - throw new Exception\InvalidArgumentException( - '$prefix must be a valid table prefix or null, empty string given' - ); - } - - $this->prefix = $prefix; + if ('' === $prefix) { + throw new Exception\InvalidArgumentException( + '$prefix must be a valid table prefix or null, empty string given' + ); } if ('' === $separator) { @@ -58,8 +40,6 @@ public function __construct( '$separator must be a valid table separator, empty string given' ); } - - $this->separator = $separator; } /**