diff --git a/docs/book/exceptions.md b/docs/book/exceptions.md new file mode 100644 index 00000000..7f6bae3b --- /dev/null +++ b/docs/book/exceptions.md @@ -0,0 +1,87 @@ +# Exceptions + +Every exception this package throws is built by a `static` named constructor on a +component-owned exception class. The message template sits beside it as a +`final public const string`. No exception is constructed with a message assembled at +the throw site. + +## The convention + +```php +namespace PhpDb\TableGateway\Exception; + +use PhpDb\Exception; + +use function sprintf; + +class RuntimeException extends Exception\RuntimeException implements ExceptionInterface +{ + final public const string NO_PRIMARY_KEY = 'No information was provided to the RowGatewayFeature' + . ' and/or no MetadataFeature could be consulted to find the primary key' + . ' necessary for RowGateway object creation.'; + + final public const string INVALID_MAGIC_CALL = 'Invalid method (%s) called, caught by %s::__call()'; + + public static function forNoPrimaryKey(): self + { + return new self(self::NO_PRIMARY_KEY); + } + + public static function forInvalidMagicCall(string $method, string $class): self + { + return new self(sprintf(self::INVALID_MAGIC_CALL, $method, $class)); + } +} +``` + +- Named constructors are `static`, return `self` and start with `for`. +- Each one has a single `final public const string` template beside it. +- The constant name mirrors the method name: `forNoPrimaryKey()` uses `NO_PRIMARY_KEY`, + `forInvalidMagicCall()` uses `INVALID_MAGIC_CALL`. Strip the `for` prefix and + upper-snake the rest. +- Use `new self(self::CONST)` when the template has no placeholder and + `new self(sprintf(self::CONST, ...))` when it does. `sprintf()` on a template with no + `%` placeholder returns its own argument, so it is not written. +- Quote a `%s` that holds a caller-supplied identifier, such as a column or parameter + name. Leave a class name, method name or count unquoted. + +The constants are `final` because the component base classes are not. Adapter packages +extend `PhpDb\Adapter\Exception\RuntimeException`, so without `final` a subclass could +redefine a published template. + +## When a condition earns its own exception class + +Naming a condition in a class instead of a method is a departure from the convention, +not an application of it. A condition earns its own class only when it carries something +a named constructor cannot: + +1. **Extra state, with accessors.** `PhpDb\Adapter\Exception\InvalidConnectionParametersException` + holds the connection parameters it rejected. +2. **An external interface contract.** `PhpDb\Exception\ContainerException` implements + PSR-11's `ContainerExceptionInterface`. +3. **A condition a consumer can act on.** `PhpDb\Adapter\Exception\InvalidQueryException` + is thrown by every adapter and caught to log the failing SQL or retry. + +Everything else is a named constructor on the component's SPL-shaped class. + +## Component namespaces + +There is exactly one marker interface, `PhpDb\Exception\ExceptionInterface`, and every +exception this package throws implements it. It extends `Throwable`, so it can be used as a +type and not only in a `catch` clause. + +Components do **not** declare their own `ExceptionInterface`. Each owns an `Exception` +namespace holding the SPL-shaped classes it needs, and those extend their counterparts in +`PhpDb\Exception`, which is where the marker is declared — so a component class inherits it +rather than restating it. A sub-namespace such as `PhpDb\Sql\Predicate\Exception` does the +same through its parent component. + +To catch every failure from one component, name its classes: + +```php +catch (PhpDb\Sql\Exception\InvalidArgumentException | PhpDb\Sql\Exception\RuntimeException $e) +``` + +A class named after an SPL exception extends that SPL exception, so `catch (\RuntimeException)` and +`catch (PhpDb\Exception\ExceptionInterface)` both behave as a reader expects. +`PhpDbTest\Exception\ExceptionHierarchyTest` enforces both rules across `src/`, for every class listed in its `EXCEPTION_CLASSES` map. diff --git a/lint-baseline.toml b/lint-baseline.toml index b2f822ad..f4f30de4 100644 --- a/lint-baseline.toml +++ b/lint-baseline.toml @@ -2868,24 +2868,6 @@ code = "prefer-test-attribute" message = "Use `#[Test]` attribute instead of `test` prefix on method `testInvokeUsesDefaultSeparatorWhenSeparatorKeyIsAbsent`." count = 1 -[[issues]] -file = "test/unit/Exception/ContainerExceptionTest.php" -code = "assertion-style" -message = "Inconsistent assertions style." -count = 4 - -[[issues]] -file = "test/unit/Exception/ContainerExceptionTest.php" -code = "prefer-test-attribute" -message = "Use `#[Test]` attribute instead of `test` prefix on method `testForServiceCreatesFormattedExceptionMessage`." -count = 1 - -[[issues]] -file = "test/unit/Exception/ContainerExceptionTest.php" -code = "prefer-test-attribute" -message = "Use `#[Test]` attribute instead of `test` prefix on method `testImplementsContainerExceptionInterface`." -count = 1 - [[issues]] file = "test/unit/ResultSet/AbstractResultSetIntegrationTest.php" code = "assertion-style" diff --git a/mkdocs.yml b/mkdocs.yml index 76af623d..186ddbfe 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -31,6 +31,7 @@ nav: - Metadata Objects: metadata/objects.md - Examples: metadata/examples.md - Profiler: profiler.md + - Exceptions: exceptions.md - "Application Integration": - "Integrating in a Laminas MVC application": application-integration/usage-in-a-laminas-mvc-application.md - "Integrating in a Mezzio application": application-integration/usage-in-a-mezzio-application.md diff --git a/src/Adapter/Exception/ErrorException.php b/src/Adapter/Exception/ErrorException.php index d2455bcf..0eef9de1 100644 --- a/src/Adapter/Exception/ErrorException.php +++ b/src/Adapter/Exception/ErrorException.php @@ -6,4 +6,4 @@ use PhpDb\Exception; -class ErrorException extends Exception\ErrorException implements ExceptionInterface {} +class ErrorException extends Exception\ErrorException {} diff --git a/src/Adapter/Exception/ExceptionInterface.php b/src/Adapter/Exception/ExceptionInterface.php deleted file mode 100644 index 43ba2552..00000000 --- a/src/Adapter/Exception/ExceptionInterface.php +++ /dev/null @@ -1,9 +0,0 @@ -executeDelete($delete); } + /** @throws Exception\RuntimeException */ public function deleteWith(Delete $delete): int { $this->initialize(); @@ -84,6 +86,7 @@ public function getColumns(): array return $this->columns; } + /** @throws Exception\RuntimeException */ public function getFeatureSet(): Feature\FeatureSet { if (! $this->isInitialized) { @@ -98,6 +101,7 @@ public function getLastInsertValue(): string|int|false|null return $this->lastInsertValue; } + /** @throws Exception\RuntimeException */ public function getResultSetPrototype(): ResultSetInterface { if (! $this->isInitialized) { @@ -107,6 +111,7 @@ public function getResultSetPrototype(): ResultSetInterface return $this->resultSetPrototype; } + /** @throws Exception\RuntimeException */ public function getSql(): Sql { if (! $this->isInitialized) { @@ -161,6 +166,7 @@ public function initialize(): void $this->isInitialized = true; } + /** @throws Exception\RuntimeException */ #[Override] public function insert(array $set): int { @@ -173,6 +179,7 @@ public function insert(array $set): int return $this->executeInsert($insert); } + /** @throws Exception\RuntimeException */ public function insertWith(Insert $insert): int { if (! $this->isInitialized) { @@ -187,6 +194,7 @@ public function isInitialized(): bool return $this->isInitialized; } + /** @throws Exception\RuntimeException */ #[Override] public function select(Where|Closure|string|array|null $where = null): ResultSetInterface { @@ -205,6 +213,7 @@ public function select(Where|Closure|string|array|null $where = null): ResultSet return $this->selectWith($select); } + /** @throws Exception\RuntimeException */ public function selectWith(Select $select): ResultSetInterface { if (! $this->isInitialized) { @@ -214,6 +223,7 @@ public function selectWith(Select $select): ResultSetInterface return $this->executeSelect($select); } + /** @throws Exception\RuntimeException */ #[Override] public function update( array $set, @@ -240,6 +250,7 @@ public function update( return $this->executeUpdate($update); } + /** @throws Exception\RuntimeException */ public function updateWith(Update $update): int { if (! $this->isInitialized) { diff --git a/src/TableGateway/Exception/ExceptionInterface.php b/src/TableGateway/Exception/ExceptionInterface.php deleted file mode 100644 index 233c7055..00000000 --- a/src/TableGateway/Exception/ExceptionInterface.php +++ /dev/null @@ -1,9 +0,0 @@ -getMessage()); - self::assertStringContainsString('Factory', $exception->getMessage()); - self::assertStringContainsString('reason', $exception->getMessage()); + self::assertInstanceOf(ContainerExceptionInterface::class, $exception); } - public function testImplementsContainerExceptionInterface(): void + #[Test] + public function forServiceIsReachableThroughThePackageMarkerInterface(): void { $exception = ContainerException::forService('Svc', 'Factory', 'reason'); - self::assertInstanceOf(ContainerExceptionInterface::class, $exception); + self::assertInstanceOf(ExceptionInterface::class, $exception); + } + + #[Test] + public function forServiceRendersItsTemplate(): void + { + $exception = ContainerException::forService('Svc', 'Factory', 'reason'); + + self::assertSame( + sprintf(ContainerException::SERVICE, 'Svc', 'Factory', 'reason'), + $exception->getMessage(), + ); } } diff --git a/test/unit/Exception/ExceptionHierarchyTest.php b/test/unit/Exception/ExceptionHierarchyTest.php new file mode 100644 index 00000000..6593f630 --- /dev/null +++ b/test/unit/Exception/ExceptionHierarchyTest.php @@ -0,0 +1,201 @@ +, class-string> + */ + private const array EXCEPTION_CLASSES = [ + AdapterException\ErrorException::class => Exception::class, + AdapterException\InvalidArgumentException::class => InvalidArgumentException::class, + AdapterException\InvalidConnectionParametersException::class => RuntimeException::class, + AdapterException\InvalidQueryException::class => UnexpectedValueException::class, + AdapterException\RuntimeException::class => RuntimeException::class, + AdapterException\UnexpectedValueException::class => UnexpectedValueException::class, + AdapterException\VunerablePlatformQuoteException::class => RuntimeException::class, + DbException\ContainerException::class => RuntimeException::class, + DbException\ErrorException::class => Exception::class, + DbException\InvalidArgumentException::class => InvalidArgumentException::class, + DbException\RuntimeException::class => RuntimeException::class, + DbException\UnexpectedValueException::class => UnexpectedValueException::class, + ResultSetException\InvalidArgumentException::class => InvalidArgumentException::class, + ResultSetException\RuntimeException::class => RuntimeException::class, + RowGatewayException\InvalidArgumentException::class => InvalidArgumentException::class, + RowGatewayException\RuntimeException::class => RuntimeException::class, + SqlException\InvalidArgumentException::class => InvalidArgumentException::class, + SqlException\RuntimeException::class => RuntimeException::class, + TableGatewayException\InvalidArgumentException::class => InvalidArgumentException::class, + TableGatewayException\RuntimeException::class => RuntimeException::class, + ]; + + /** + * Exempt from the naming guard: its factory is named for its arguments rather than the + * fault, and holds its message inline. + */ + private const string UNCONVERTED_CLASS = AdapterException\VunerablePlatformQuoteException::class; + + /** @return array */ + public static function exceptionClassProvider(): array + { + $cases = []; + foreach (self::EXCEPTION_CLASSES as $class => $splParent) { + $cases[$class] = [$class, $splParent]; + } + + ksort($cases); + + return $cases; + } + + /** @return array */ + public static function namedConstructorProvider(): array + { + $cases = []; + foreach (array_keys(self::EXCEPTION_CLASSES) as $class) { + if (self::UNCONVERTED_CLASS === $class) { + continue; + } + + foreach (self::namedConstructors($class) as $method) { + $cases["{$class}::{$method}()"] = [ + $class, + $method, + self::templateConstantFor($method), + ]; + } + } + + ksort($cases); + + return $cases; + } + + /** + * The named constructors a class declares itself. + * + * @param class-string $class + * @return list + */ + private static function namedConstructors(string $class): array + { + $methods = []; + foreach ((new ReflectionClass($class))->getMethods(ReflectionMethod::IS_STATIC) as $method) { + $name = $method->getName(); + if ($method->getDeclaringClass()->getName() !== $class || ! str_starts_with($name, 'for')) { + continue; + } + + $methods[] = $name; + } + + return $methods; + } + + /** + * The template constant a named constructor's name implies. + */ + private static function templateConstantFor(string $method): string + { + $condition = substr( + string: $method, + offset: 3, + ); + + return strtoupper((string) preg_replace( + pattern: '/(?getConstants(), + "{$class}::{$method}() has no {$expectedConstant} template constant", + ); + } + + #[Test] + public function theMarkerInterfaceIsAThrowableSubtype(): void + { + self::assertTrue( + (new ReflectionClass(DbException\ExceptionInterface::class))->isSubclassOf(Throwable::class), + DbException\ExceptionInterface::class . ' must extend Throwable, or it cannot be used as a type', + ); + } +}