diff --git a/CHANGELOG.md b/CHANGELOG.md index 817e9d4..50f812b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Changed + +- `Origin` now accepts `$appDir` and `$vendorDir` parameters directly, instead of `Root` object. This makes + application detection more explicit and reliable, without depending on additional packages. +- `Fingerprint` now accepts an `$appDir` parameter, instead of `Root` object. This makes relative path fingerprinting + more explicitly opt-in. + ## [0.1.0] - 2026-10-05 ### Added diff --git a/docs/problems.md b/docs/problems.md index 91da0de..bc0c90d 100644 --- a/docs/problems.md +++ b/docs/problems.md @@ -70,6 +70,23 @@ followed by extension members in insertion order. The HTTP problem documents described below will also throw when attempting to overwrite `title` or `status`. +## ProblemMap + +`Errata\ProblemMap` maps a class name to a factory `Closure(object): Problem`: + +```php +use Errata\Problem; +use Errata\ProblemMap; + +$map = new ProblemMap([ + ValidationError::class => new ValidationProblemFactory(), +]); +``` + +Lookup checks the exact class first, then parent classes nearest-first, then implemented interfaces, and returns +the first factory found. When nothing matches, `get()` returns `null` and the transformer falls back to an +`InternalServerError`. + ## HTTP problems The `Errata\Http` namespace ships fixed-status problem documents. `Errata\Http\HttpProblem` is the abstract base; diff --git a/docs/throwables.md b/docs/throwables.md index 9743add..c4c63c2 100644 --- a/docs/throwables.md +++ b/docs/throwables.md @@ -22,15 +22,16 @@ and an `ExtensionList` whose extensions enrich the resulting problem. ## ProblemMap -`Errata\ProblemMap` maps a throwable class name to a factory `Closure(object): Problem`. +`Errata\ProblemMap` maps a class name to a factory `Closure(object): Problem`. This can be used for throwables: ```php use Errata\Http\Client\UnprocessableContent; use Errata\ProblemMap; $map = new ProblemMap([ - ValidationFailed::class => static fn(ValidationFailed $e): UnprocessableContent - => new UnprocessableContent(detail: $e->getMessage()), + ValidationFailed::class => static fn(ValidationFailed $e): UnprocessableContent => new UnprocessableContent(detail: $e->getMessage()), + // Or, if you prefer an object with __invoke: + ValidationFailed::class => new UnprocessableContentFactory(), ]); ``` @@ -98,17 +99,12 @@ Additional constructor options: - `Fingerprint` takes `algo`, which defaults to `xxh64` and accepts any `hash()` algorithm, and a `Root`. The hash input joins the throwable class, code, and the file path made relative to the application root, so fingerprints are stable across systems. -- `Origin` and `Fingerprint` both take a `Root`. +- `Origin` takes `appDir` and `vendorDir` to allow picking an application-specific origin by finding the first + `Location` that is inside the `appDir` and *not* inside the `vendorDir`. +- `Fingerprint` takes `appDir` to make the file path relative for increased fingerprint stability. ### Trace locations `Errata\Trace\Location` represents one entry in a throwable's stack trace. A location list starts with the throwable's own file and line, followed by the trace frames, skipping entries without a file. Locations serialize to JSON as `"file:line"` strings. - -## Root - -`Errata\Root` identifies the application directory and the vendor directory beneath it. When no directory is -given, it is detected from Composer. `isApp()` reports whether a path is inside the application but not the vendor -tree, and `relative()` makes a path relative to the application root while leaving paths outside it untouched. -`Origin` and `Fingerprint` use both. diff --git a/src/Extension/Fingerprint.php b/src/Extension/Fingerprint.php index 585beb6..1d5cdd5 100644 --- a/src/Extension/Fingerprint.php +++ b/src/Extension/Fingerprint.php @@ -5,34 +5,50 @@ namespace Errata\Extension; use Errata\Problem; -use Errata\Root; use Override; use Throwable; use function hash; +use function Psl\Filesystem\canonicalize; +use function Psl\Str\Byte\after; use function Psl\Str\join; +use const Psl\Filesystem\SEPARATOR; + /** * @api */ final readonly class Fingerprint implements Extension { + /** @var non-empty-string|null */ + private ?string $appDir; + + /** + * @param string|null $appDir + */ public function __construct( - private Root $root = new Root(), /** @var non-empty-string */ private string $key = 'fingerprint', /** @var non-empty-string */ private string $algo = 'xxh64', - ) {} + ?string $appDir = null, + ) { + $this->appDir = $appDir ? canonicalize($appDir) : null; + } #[Override] public function extend(Problem $problem, Throwable $throwable): void { + $file = $throwable->getFile(); + + if ($this->appDir) { + $file = after($file, $this->appDir . SEPARATOR) ?? $file; + } + $elements = [ $throwable::class, (string) $throwable->getCode(), - // File path MUST be made relative for stability across systems. - $this->root->relative($throwable->getFile()), + $file, ]; $problem->extend($this->key, hash($this->algo, join($elements, glue: '|'))); diff --git a/src/Extension/Origin.php b/src/Extension/Origin.php index 01600f5..6f4eaa9 100644 --- a/src/Extension/Origin.php +++ b/src/Extension/Origin.php @@ -5,26 +5,48 @@ namespace Errata\Extension; use Errata\Problem; -use Errata\Root; use Errata\Trace\Location; use Override; use Throwable; +use function Psl\Filesystem\canonicalize; +use function Psl\Str\Byte\starts_with; + /** * @api */ final readonly class Origin implements Extension { + /** @var non-empty-string|null */ + private ?string $appDir; + + /** @var non-empty-string|null */ + private ?string $vendorDir; + + /** + * @param string|null $appDir + * @param string|null $vendorDir + */ public function __construct( - private Root $root = new Root(), /** @var non-empty-string */ private string $key = 'origin', - ) {} + ?string $appDir = null, + ?string $vendorDir = null, + ) { + $this->appDir = $appDir ? canonicalize($appDir) : null; + $this->vendorDir = $vendorDir ? canonicalize($vendorDir) : null; + } #[Override] public function extend(Problem $problem, Throwable $throwable): void { - $problem->extend($this->key, $this->findOrigin($throwable)); + $origin = $this->findOrigin($throwable); + + if ($this->appDir) { + $origin = $origin->withoutDirectory($this->appDir); + } + + $problem->extend($this->key, $origin); } private function findOrigin(Throwable $throwable): Location @@ -32,11 +54,24 @@ private function findOrigin(Throwable $throwable): Location $locations = Location::trace($throwable); foreach ($locations as $location) { - if ($this->root->isApp($location->file)) { + if ($this->isApp($location)) { return $location; } } return $locations[0] ?? new Location('(internal)'); } + + private function isApp(Location $location): bool + { + if ($this->appDir === null) { + return false; + } + + if ($this->vendorDir && starts_with($location->file, $this->vendorDir)) { + return false; + } + + return starts_with($location->file, $this->appDir); + } } diff --git a/src/Problem.php b/src/Problem.php index 2c4729b..3719c54 100644 --- a/src/Problem.php +++ b/src/Problem.php @@ -78,27 +78,13 @@ final public function extend(string $name, mixed $value): self **/ final public function toArray(): array { - $members = []; - - if ($this->type !== null) { - $members['type'] = $this->type; - } - - if ($this->title !== null) { - $members['title'] = $this->title; - } - - if ($this->detail !== null) { - $members['detail'] = $this->detail; - } - - if ($this->status !== null) { - $members['status'] = $this->status; - } - - if ($this->instance !== null) { - $members['instance'] = $this->instance; - } + $members = [ + 'type' => $this->type, + 'title' => $this->title, + 'detail' => $this->detail, + 'status' => $this->status, + 'instance' => $this->instance, + ]; return filter_nulls($members + $this->extensions); } diff --git a/src/ProblemMap.php b/src/ProblemMap.php index 69ba7ed..ba488d3 100644 --- a/src/ProblemMap.php +++ b/src/ProblemMap.php @@ -4,8 +4,6 @@ namespace Errata; -use Closure; - use function class_implements; use function class_parents; @@ -13,7 +11,7 @@ * @api * @template T of object * @template Tk of class-string - * @template Tv of Closure(T):Problem + * @template Tv of callable(T):Problem */ final readonly class ProblemMap { @@ -23,9 +21,10 @@ public function __construct( ) {} /** + * @param T $instance * @return Tv|null */ - public function get(object $instance): ?Closure + public function get(object $instance): ?callable { $classes = [ $instance::class, diff --git a/src/Root.php b/src/Root.php deleted file mode 100644 index f647a92..0000000 --- a/src/Root.php +++ /dev/null @@ -1,75 +0,0 @@ -dir = Path::normalize($dir); - $this->vendor = Path::canonicalize($vendor, $this->dir); - } - - public function isApp(string $path): bool - { - if ($path === '' || $path === '/' || $path === SEPARATOR) { - return false; - } - - $path = Path::normalize($path); - - if (starts_with($path, $this->vendor)) { - return false; - } - - return starts_with($path, $this->dir); - } - - public function relative(string $path): string - { - if ($path === '' || $path === $this->dir) { - return ''; - } - - $path = Path::normalize($path); - $prefix = "{$this->dir}/"; - - if (starts_with($path, $prefix)) { - return after($path, $prefix) ?? ''; - } - - return $path; - } -} diff --git a/src/Trace/Location.php b/src/Trace/Location.php index b4de60c..3499f2f 100644 --- a/src/Trace/Location.php +++ b/src/Trace/Location.php @@ -9,6 +9,11 @@ use Stringable; use Throwable; +use function Psl\Str\Byte\after; +use function Psl\Str\Byte\starts_with; + +use const Psl\Filesystem\SEPARATOR; + /** * @internal */ @@ -47,6 +52,20 @@ public function __construct( public int $line = 0, ) {} + /** + * @param non-empty-string $dir + */ + public function withoutDirectory(string $dir): self + { + if (!starts_with($this->file, $dir)) { + return $this; + } + + // @mago-expect analysis:possibly-invalid-argument + // @mago-expect analysis:possibly-null-argument + return new self(after($this->file, $dir . SEPARATOR), $this->line); + } + public function toString(): string { if ($this->line < 1) { diff --git a/tests/Extension/FingerprintTest.php b/tests/Extension/FingerprintTest.php index 71c7ef6..fab20bf 100644 --- a/tests/Extension/FingerprintTest.php +++ b/tests/Extension/FingerprintTest.php @@ -6,7 +6,6 @@ use Errata\Extension\Fingerprint; use Errata\Problem; -use Errata\Root; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; use RuntimeException; @@ -20,10 +19,35 @@ final class FingerprintTest extends TestCase public function testItExtendsTheProblemWithAFingerprint(): void { $problem = new Problem(); + $exception = new RuntimeException('boom', 42); - new Fingerprint()->extend($problem, new RuntimeException('boom', 42)); + $parts = [ + $exception::class, + (string) $exception->getCode(), + $exception->getFile(), + ]; - $expected = hash('xxh64', join([RuntimeException::class, '42', new Root()->relative(__FILE__)], glue: '|')); + new Fingerprint()->extend($problem, $exception); + + $expected = hash('xxh64', join($parts, glue: '|')); + + $this->assertSame($expected, $problem->extensions['fingerprint'] ?? null); + } + + public function testItMakesTheFilePathRelativeWithAppDir(): void + { + $problem = new Problem(); + $exception = new RuntimeException('boom', 42); + + $parts = [ + $exception::class, + (string) $exception->getCode(), + 'tests/Extension/FingerprintTest.php', // __FILE__ made relative by appDir + ]; + + new Fingerprint(appDir: __DIR__ . '/../..')->extend($problem, $exception); + + $expected = hash('xxh64', join($parts, glue: '|')); $this->assertSame($expected, $problem->extensions['fingerprint'] ?? null); } @@ -31,10 +55,17 @@ public function testItExtendsTheProblemWithAFingerprint(): void public function testItUsesACustomKeyAndAlgorithm(): void { $problem = new Problem(); + $exception = new RuntimeException('boom', 42); + + $parts = [ + $exception::class, + (string) $exception->getCode(), + $exception->getFile(), + ]; - new Fingerprint(key: 'hash', algo: 'crc32b')->extend($problem, new RuntimeException('boom', 42)); + new Fingerprint(key: 'hash', algo: 'crc32b')->extend($problem, $exception); - $expected = hash('crc32b', join([RuntimeException::class, '42', new Root()->relative(__FILE__)], glue: '|')); + $expected = hash('crc32b', join($parts, glue: '|')); $this->assertSame($expected, $problem->extensions['hash'] ?? null); } diff --git a/tests/Extension/OriginTest.php b/tests/Extension/OriginTest.php index 0cfac51..fb102d1 100644 --- a/tests/Extension/OriginTest.php +++ b/tests/Extension/OriginTest.php @@ -6,7 +6,6 @@ use Errata\Extension\Origin; use Errata\Problem; -use Errata\Root; use Errata\Trace\Location; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; @@ -26,20 +25,34 @@ public function testItExtendsTheProblemWithTheFirstAppLocation(): void $origin = $problem->extensions['origin'] ?? null; $this->assertInstanceOf(Location::class, $origin); - $this->assertSame(__FILE__, $origin->file); } public function testItFallsBackToTheFirstLocationOutsideTheApp(): void { $problem = new Problem(); - new Origin(new Root('/tmp'))->extend($problem, $this->catchThrowable()); + new Origin(appDir: '/tmp')->extend($problem, $this->catchThrowable()); /** @var Location|null $origin */ $origin = $problem->extensions['origin'] ?? null; $this->assertInstanceOf(Location::class, $origin); - $this->assertSame(__FILE__, $origin->file); + } + + public function testItIgnoresVendorLocations(): void + { + $problem = new Problem(); + + new Origin(appDir: __DIR__ . '/../..', vendorDir: __DIR__ . '/../../vendor')->extend( + $problem, + $this->catchThrowable(), + ); + + /** @var Location|null $origin */ + $origin = $problem->extensions['origin'] ?? null; + + $this->assertInstanceOf(Location::class, $origin); + $this->assertSame('tests/Extension/OriginTest.php', $origin->file); } public function testItUsesACustomKey(): void @@ -54,7 +67,8 @@ public function testItUsesACustomKey(): void private function catchThrowable(): Throwable { try { - throw new RuntimeException('Fixture exception'); + $this->throwException(new RuntimeException('Fixture exception')); + $this->fail('Should have thrown exception'); } catch (Throwable $throwable) { return $throwable; } diff --git a/tests/Fixtures/NotificationProblemFactory.php b/tests/Fixtures/NotificationProblemFactory.php new file mode 100644 index 0000000..a0089e6 --- /dev/null +++ b/tests/Fixtures/NotificationProblemFactory.php @@ -0,0 +1,18 @@ +getMessage()); + } +} diff --git a/tests/ProblemMapTest.php b/tests/ProblemMapTest.php index 75ec39a..ff2f44c 100644 --- a/tests/ProblemMapTest.php +++ b/tests/ProblemMapTest.php @@ -7,6 +7,7 @@ use Errata\Problem; use Errata\ProblemMap; use Errata\Tests\Fixtures\NotificationException; +use Errata\Tests\Fixtures\NotificationProblemFactory; use Errata\Tests\Fixtures\Recoverable; use Exception; use PHPUnit\Framework\Attributes\CoversClass; @@ -41,6 +42,14 @@ public function testReturnsTheFactoryForTheExactClass(): void $this->assertSame($factory, $map->get(new NotificationException())); } + public function testReturnsTheFactoryObjectForTheExactClass(): void + { + $factory = new NotificationProblemFactory(); + $map = new ProblemMap([NotificationException::class => $factory]); + + $this->assertSame($factory, $map->get(new NotificationException())); + } + public function testReturnsTheFactoryForAParentClass(): void { $factory = static fn(NotificationException $_instance): Problem => new Problem(); diff --git a/tests/RootTest.php b/tests/RootTest.php deleted file mode 100644 index 295a966..0000000 --- a/tests/RootTest.php +++ /dev/null @@ -1,142 +0,0 @@ -root = get_directory(__DIR__); - } - - public function testItDetectsTheRootPath(): void - { - $root = new Root(); - - $this->assertSame(canonicalize($this->root), $root->dir); - } - - public function testItCanonicalizesTheRootDirectory(): void - { - $root = new Root(dir: __DIR__ . '/../'); - - $this->assertSame(canonicalize($this->root), $root->dir); - } - - public function testItRequiresTheRootDirectoryToExist(): void - { - $this->expectException(InvariantViolationException::class); - $this->expectExceptionMessageIs('Root directory must exist'); - - new Root('/tmp/errata/non-existant-directory'); - } - - public function testItResolvesTheDefaultVendorDirectory(): void - { - $root = new Root($this->root); - - $this->assertSame("{$this->root}/vendor", $root->vendor); - } - - public function testItResolvesAnExistingVendorDirectory(): void - { - $root = new Root($this->root, vendor: 'tests'); - - $this->assertSame("{$this->root}/tests", $root->vendor); - } - - public function testItAllowsVendorDirectoryToNotExist(): void - { - $root = new Root($this->root, vendor: 'missing'); - - $this->assertSame("{$this->root}/missing", $root->vendor); - } - - public function testItResolvesVendorPathSegments(): void - { - $root = new Root($this->root, vendor: 'foo/./bar/../baz/'); - - $this->assertSame("{$this->root}/foo/baz", $root->vendor); - } - - public function testItResolvesAnAbsoluteVendorDirectory(): void - { - $root = new Root($this->root, vendor: '/tmp/vendor/'); - - $this->assertSame('/tmp/vendor', $root->vendor); - } - - public function testItDoesNotEscapeTheFilesystemRoot(): void - { - $root = new Root($this->root, vendor: '/../vendor'); - - $this->assertSame('/vendor', $root->vendor); - } - - public function testItMakesRelativePaths(): void - { - $root = new Root($this->root); - - $this->assertSame('', $root->relative('')); - $this->assertSame('', $root->relative($this->root)); - $this->assertSame('tests', $root->relative(__DIR__)); - $this->assertSame('tests/RootTest.php', $root->relative(__FILE__)); - } - - public function testItDoesNotModifyPathsOutside(): void - { - $root = new Root($this->root); - - $this->assertSame('/tmp', $root->relative('/tmp')); - } - - public function testItDetectsAppPaths(): void - { - $root = new Root($this->root); - - $this->assertTrue($root->isApp($this->root)); - $this->assertTrue($root->isApp($this->root . '/src/Root.php')); - } - - public function testItRejectsVendorPaths(): void - { - $root = new Root($this->root); - - $this->assertFalse($root->isApp($root->vendor)); - $this->assertFalse($root->isApp($this->root . '/vendor/autoload.php')); - } - - public function testItRejectsPathsOutsideTheRoot(): void - { - $root = new Root($this->root); - - $this->assertFalse($root->isApp('/tmp/foo.php')); - } - - public function testItRejectsEmptyPaths(): void - { - $root = new Root($this->root); - - $this->assertFalse($root->isApp('')); - $this->assertFalse($root->isApp('/')); - $this->assertFalse($root->isApp(SEPARATOR)); - } -} diff --git a/tests/Trace/LocationTest.php b/tests/Trace/LocationTest.php index 73327cd..0455c14 100644 --- a/tests/Trace/LocationTest.php +++ b/tests/Trace/LocationTest.php @@ -42,6 +42,15 @@ public function testItDefaultsTheLineToZero(): void $this->assertSame('/app/src/File.php', $location->toString()); } + public function testItRemovesTheDirectoryPrefix(): void + { + $valid = new Location('/app/src/File.php')->withoutDirectory('/app'); + $invalid = new Location('/app/src/File.php')->withoutDirectory('/tmp'); + + $this->assertSame('src/File.php', $valid->toString()); + $this->assertSame('/app/src/File.php', $invalid->toString()); + } + public function testItTracesTheThrowableAndItsFrames(): void { $throwable = $this->catchThrowable($this->throwFromHelper(...));