diff --git a/README.md b/README.md index 61a557d..9f60b8a 100644 --- a/README.md +++ b/README.md @@ -52,6 +52,36 @@ container; the remaining parameters are filled by type and defaults: $args = $resolver->resolve($constructor, ['username' => 'admin']); ``` +### Memoize parameter introspection + +Reflection is expensive. The resolver accepts an optional PSR-16 cache as its second argument; +when supplied, the per-parameter spec (name, type, variadic flag, default-available flag) is +memoized under a stable key derived from the callable identity (`FQCN::method` for methods, +`fn:name` for named functions), so repeated `resolve()` calls on different +`ReflectionMethod` / `ReflectionFunction` instances of the same callable share one spec and +skip `ReflectionParameter` method calls entirely. Closures and invocable objects have no stable +identity across reflections and bypass the cache. + +```php +use Respect\Parameter\Resolver; + +$resolver = new Resolver($container, $psr16Cache); +``` + +The package ships with a ready-to-use in-memory PSR-16 implementation so you get the memoization +benefit with no external dependency: + +```php +use Respect\Parameter\InMemoryCache; +use Respect\Parameter\Resolver; + +$resolver = new Resolver($container, new InMemoryCache()); +``` + +`InMemoryCache` is a process-local array-backed cache: entries live for the lifetime of the +cache instance and are not shared across processes. For longer-lived or shared caching, pass any +real PSR-16 implementation (Symfony Cache, PSR-16 adapter over APCu, etc.). + ### Bind to the interface Type-hint `ParameterResolver` (the `resolve()` contract) rather than the concrete `Resolver` to stay @@ -98,6 +128,9 @@ Resolver::acceptsType($reflection, LoggerInterface::class); // true/false `Resolver` implements `ParameterResolver`. +`InMemoryCache` implements `Psr\SimpleCache\CacheInterface` and is the bundled zero-dependency +PSR-16 cache for memoizing the resolver's parameter spec. + ## License ISC. See [LICENSE](LICENSE). diff --git a/composer.json b/composer.json index 158654a..a019373 100644 --- a/composer.json +++ b/composer.json @@ -12,7 +12,8 @@ ], "require": { "php": "^8.5", - "psr/container": "^2.0" + "psr/container": "^2.0", + "psr/simple-cache": "^3.0" }, "require-dev": { "phpstan/phpstan": "^2.1", diff --git a/composer.lock b/composer.lock index 30d1a47..b41f67c 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "ddbf5b208b5273244203e5fb86ee1cc7", + "content-hash": "44496d3c894f5ae3bec6e8f505b7d215", "packages": [ { "name": "psr/container", @@ -58,6 +58,57 @@ "source": "https://github.com/php-fig/container/tree/2.0.2" }, "time": "2021-11-05T16:47:00+00:00" + }, + { + "name": "psr/simple-cache", + "version": "3.0.0", + "source": { + "type": "git", + "url": "https://github.com/php-fig/simple-cache.git", + "reference": "764e0b3939f5ca87cb904f570ef9be2d78a07865" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/php-fig/simple-cache/zipball/764e0b3939f5ca87cb904f570ef9be2d78a07865", + "reference": "764e0b3939f5ca87cb904f570ef9be2d78a07865", + "shasum": "" + }, + "require": { + "php": ">=8.0.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "3.0.x-dev" + } + }, + "autoload": { + "psr-4": { + "Psr\\SimpleCache\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "PHP-FIG", + "homepage": "https://www.php-fig.org/" + } + ], + "description": "Common interfaces for simple caching", + "keywords": [ + "cache", + "caching", + "psr", + "psr-16", + "simple-cache" + ], + "support": { + "source": "https://github.com/php-fig/simple-cache/tree/3.0.0" + }, + "time": "2021-10-29T13:26:27+00:00" } ], "packages-dev": [ diff --git a/src/InMemoryCache.php b/src/InMemoryCache.php new file mode 100644 index 0000000..50f9bbb --- /dev/null +++ b/src/InMemoryCache.php @@ -0,0 +1,115 @@ + */ + protected array $store = []; + + public function get(string $key, mixed $default = null): mixed + { + return array_key_exists($key, $this->store) ? $this->store[$key] : $default; + } + + public function set(string $key, mixed $value, DateInterval|int|null $ttl = null): bool + { + $this->store[$key] = $value; + + return true; + } + + public function delete(string $key): bool + { + unset($this->store[$key]); + + return true; + } + + public function clear(): bool + { + $this->store = []; + + return true; + } + + /** + * @param iterable $keys + * + * @return iterable + */ + public function getMultiple(iterable $keys, mixed $default = null): iterable + { + $out = []; + foreach ($keys as $key) { + $out[$key] = $this->get($key, $default); + } + + return $out; + } + + /** + * @param iterable $values + * + * @throws InvalidArgumentException When a key is not a non-empty string. + */ + public function setMultiple(iterable $values, DateInterval|int|null $ttl = null): bool + { + foreach ($values as $key => $value) { + /** @phpstan-ignore function.alreadyNarrowedType (defensive: callers may pass int-keyed arrays that PHP upcasts to int on iteration) */ + if (!is_string($key) || $key === '') { + throw new InvalidCacheKey('Cache keys must be non-empty strings.'); + } + + $this->store[$key] = $value; + } + + return true; + } + + /** @param iterable $keys */ + public function deleteMultiple(iterable $keys): bool + { + foreach ($keys as $key) { + unset($this->store[$key]); + } + + return true; + } + + public function has(string $key): bool + { + return array_key_exists($key, $this->store); + } +} diff --git a/src/InvalidCacheKey.php b/src/InvalidCacheKey.php new file mode 100644 index 0000000..21645c6 --- /dev/null +++ b/src/InvalidCacheKey.php @@ -0,0 +1,26 @@ +getParameters(); - if ($parameters === []) { + $spec = $this->specFor($reflection); + if ($spec === []) { return array_values($arguments); } @@ -72,20 +79,28 @@ public function resolve(ReflectionFunctionAbstract $reflection, array $arguments } } + if ( + $named === [] + && count($positional) === count($spec) + && self::allPositionalsAlign($spec, $positional, $this->container) + ) { + return $positional; + } + $resolved = []; - $index = 0; + $posIndex = 0; $count = count($positional); - foreach ($parameters as $param) { - $name = $param->getName(); + foreach ($spec as $paramIndex => $param) { + $name = $param['name']; - if ($param->isVariadic()) { + if ($param['variadic']) { if (array_key_exists($name, $named)) { $resolved[] = $named[$name]; } - while ($index < $count) { - $resolved[] = $positional[$index++]; + while ($posIndex < $count) { + $resolved[] = $positional[$posIndex++]; } break; @@ -97,10 +112,10 @@ public function resolve(ReflectionFunctionAbstract $reflection, array $arguments continue; } - $type = self::typeName($param); + $type = $param['type']; - if ($type !== null && isset($positional[$index]) && $positional[$index] instanceof $type) { - $resolved[] = $positional[$index++]; + if ($type !== null && isset($positional[$posIndex]) && $positional[$posIndex] instanceof $type) { + $resolved[] = $positional[$posIndex++]; continue; } @@ -111,10 +126,10 @@ public function resolve(ReflectionFunctionAbstract $reflection, array $arguments continue; } - if ($index < $count) { - $resolved[] = $positional[$index++]; - } elseif ($param->isDefaultValueAvailable()) { - $resolved[] = $param->getDefaultValue(); + if ($posIndex < $count) { + $resolved[] = $positional[$posIndex++]; + } elseif ($param['hasDefault']) { + $resolved[] = $reflection->getParameters()[$paramIndex]->getDefaultValue(); } else { $resolved[] = null; } @@ -186,4 +201,93 @@ private static function typeName(ReflectionParameter $param): string|null return $type instanceof ReflectionNamedType && !$type->isBuiltin() ? $type->getName() : null; /* Ignore Reason: !isBuiltin() guarantees class-string */ } + + /** @return list */ + private function specFor(ReflectionFunctionAbstract $reflection): array + { + $key = self::CACHE_KEY_PREFIX . md5($this->cacheKey($reflection)); + + try { + /** @var list $cached */ + $cached = $this->specCache->get($key); + } catch (InvalidArgumentException) { + $cached = null; + } + + if (is_array($cached)) { + return $cached; + } + + $spec = $this->buildSpec($reflection); + + try { + $this->specCache->set($key, $spec); + } catch (InvalidArgumentException) { + } + + return $spec; + } + + /** @return non-empty-string */ + private function cacheKey(ReflectionFunctionAbstract $reflection): string + { + if ($reflection instanceof ReflectionMethod) { + return $reflection->getDeclaringClass()->getName() . '::' . $reflection->getName(); + } + + if ($reflection instanceof ReflectionFunction && $reflection->isClosure()) { + return $reflection->getFileName() . '::' . $reflection->getStartLine(); + } + + return $reflection->getName(); + } + + /** @return list */ + private function buildSpec(ReflectionFunctionAbstract $reflection): array + { + $spec = []; + foreach ($reflection->getParameters() as $param) { + $spec[] = [ + 'name' => $param->getName(), + 'type' => self::typeName($param), + 'variadic' => $param->isVariadic(), + 'hasDefault' => $param->isDefaultValueAvailable(), + ]; + } + + return $spec; + } + + /** + * @param list $spec + * @param list $positional + */ + private static function allPositionalsAlign(array $spec, array $positional, ContainerInterface $container): bool + { + foreach ($spec as $i => $param) { + if ($param['variadic']) { + // A variadic consumes "the rest"; the count precondition + // (count($positional) === count($spec)) would leave exactly + // one positional for it, which is technically safe here, but + // bailing keeps the proof simple and the cost is negligible. + return false; + } + + $type = $param['type']; + + if ($type === null) { + continue; + } + + if (isset($positional[$i]) && $positional[$i] instanceof $type) { + continue; + } + + if ($container->has($type)) { + return false; + } + } + + return true; + } } diff --git a/tests/fixtures/ArrayCache.php b/tests/fixtures/ArrayCache.php new file mode 100644 index 0000000..9a47dc8 --- /dev/null +++ b/tests/fixtures/ArrayCache.php @@ -0,0 +1,60 @@ +store)) { + ++$this->hits; + + return $this->store[$key]; + } + + ++$this->misses; + + return $default; + } + + public function set(string $key, mixed $value, DateInterval|int|null $ttl = null): bool + { + ++$this->sets; + + return parent::set($key, $value, $ttl); + } + + /** @param iterable $values */ + public function setMultiple(iterable $values, DateInterval|int|null $ttl = null): bool + { + // Delegates to parent::set so we increment $this->sets per entry exactly + // once (parent::setMultiple writes the store directly, bypassing set()). + foreach ($values as $key => $value) { + $this->set((string) $key, $value, $ttl); + } + + return true; + } +} diff --git a/tests/fixtures/ConsumerWithExpensiveDefault.php b/tests/fixtures/ConsumerWithExpensiveDefault.php new file mode 100644 index 0000000..291f59a --- /dev/null +++ b/tests/fixtures/ConsumerWithExpensiveDefault.php @@ -0,0 +1,24 @@ + + */ + +declare(strict_types=1); + +namespace Respect\Parameter\Test\Fixtures; + +final class TwoRequiredConsumer +{ + public function __construct( + public readonly SampleService $service, + public readonly string $value, + ) { + } +} diff --git a/tests/unit/InMemoryCacheTest.php b/tests/unit/InMemoryCacheTest.php new file mode 100644 index 0000000..3434eb9 --- /dev/null +++ b/tests/unit/InMemoryCacheTest.php @@ -0,0 +1,254 @@ +get('missing', 'fallback')); + } + + #[Test] + public function itShouldReturnNullByDefaultWhenNoDefaultGiven(): void + { + self::assertNull((new InMemoryCache())->get('missing')); + } + + #[Test] + public function itShouldStoreAndRetrieveScalarValues(): void + { + $cache = new InMemoryCache(); + + $cache->set('answer', 42); + + self::assertSame(42, $cache->get('answer')); + } + + #[Test] + public function itShouldStoreAndRetrieveComplexValues(): void + { + $cache = new InMemoryCache(); + $object = new stdClass(); + $object->name = 'Alice'; + + $cache->set('user', $object); + $cache->set('list', [1, 2, 3]); + $cache->set('null', null); + + self::assertSame($object, $cache->get('user')); + self::assertSame([1, 2, 3], $cache->get('list')); + self::assertNull($cache->get('null')); + // Distinguish stored null from absent key via has() + self::assertTrue($cache->has('null')); + self::assertFalse($cache->has('absent')); + } + + #[Test] + public function itShouldOverwritePreviousValueOnSet(): void + { + $cache = new InMemoryCache(); + + $cache->set('key', 'first'); + $cache->set('key', 'second'); + + self::assertSame('second', $cache->get('key')); + } + + #[Test] + public function itShouldReportWhetherKeyExistsViaHas(): void + { + $cache = new InMemoryCache(); + $cache->set('present', 'value'); + + self::assertTrue($cache->has('present')); + self::assertFalse($cache->has('absent')); + } + + #[Test] + public function itShouldDeleteSingleKey(): void + { + $cache = new InMemoryCache(); + $cache->set('keep', 'a'); + $cache->set('drop', 'b'); + + $deleted = $cache->delete('drop'); + + self::assertTrue($deleted); + self::assertTrue($cache->has('keep')); + self::assertFalse($cache->has('drop')); + } + + #[Test] + public function itShouldReturnTrueWhenDeletingAbsentKey(): void + { + self::assertTrue((new InMemoryCache())->delete('never-set')); + } + + #[Test] + public function itShouldClearAllEntries(): void + { + $cache = new InMemoryCache(); + $cache->set('one', 1); + $cache->set('two', 2); + + $cleared = $cache->clear(); + + self::assertTrue($cleared); + self::assertFalse($cache->has('one')); + self::assertFalse($cache->has('two')); + } + + #[Test] + public function itShouldGetMultipleValuesWithDefaultForMissing(): void + { + $cache = new InMemoryCache(); + $cache->set('a', 1); + $cache->set('c', 3); + + $out = $cache->getMultiple(['a', 'b', 'c'], 'missing'); + + self::assertSame(['a' => 1, 'b' => 'missing', 'c' => 3], iterator_to_array($out, true)); + } + + #[Test] + public function itShouldSetMultipleValues(): void + { + $cache = new InMemoryCache(); + + $result = $cache->setMultiple(['x' => 10, 'y' => 20, 'z' => 30]); + + self::assertTrue($result); + self::assertSame(10, $cache->get('x')); + self::assertSame(20, $cache->get('y')); + self::assertSame(30, $cache->get('z')); + } + + #[Test] + public function itShouldDeleteMultipleKeys(): void + { + $cache = new InMemoryCache(); + $cache->setMultiple(['a' => 1, 'b' => 2, 'c' => 3]); + + $result = $cache->deleteMultiple(['a', 'c']); + + self::assertTrue($result); + self::assertFalse($cache->has('a')); + self::assertTrue($cache->has('b')); + self::assertFalse($cache->has('c')); + } + + #[Test] + public function itShouldAcceptTtlOnSetWithoutExpiring(): void + { + // TTL is accepted for PSR-16 conformance but ignored (process-local cache). + $cache = new InMemoryCache(); + + $cache->set('key', 'value', 60); + $cache->set('key2', 'value2', new DateInterval('PT60S')); + + self::assertSame('value', $cache->get('key')); + self::assertSame('value2', $cache->get('key2')); + } + + #[Test] + public function itShouldThrowWhenSetMultipleReceivesNonStringKey(): void + { + $cache = new InMemoryCache(); + + $this->expectException(InvalidCacheKey::class); + + // int keys (PHP's natural array iteration) must be rejected as PSR-16 keys. + /** @phpstan-ignore argument.type (deliberately invalid input for this throw test) */ + $cache->setMultiple([1 => 'a', 2 => 'b']); + } + + #[Test] + public function itShouldThrowWhenSetMultipleReceivesEmptyStringKey(): void + { + $cache = new InMemoryCache(); + + $this->expectException(InvalidCacheKey::class); + + $cache->setMultiple(['' => 'a']); + } + + #[Test] + public function itShouldThrowPsr16CompliantInvalidArgumentInstance(): void + { + // The concrete InvalidCacheKey must implement Psr\SimpleCache\InvalidArgumentException + // so callers catching the PSR-16 interface see it. + $cache = new InMemoryCache(); + + try { + /** @phpstan-ignore argument.type (deliberately invalid input for this throw test) */ + $cache->setMultiple([1 => 'a']); + self::fail('Expected InvalidCacheKey to be thrown'); + } catch (InvalidCacheKey $e) { + self::assertInstanceOf(PsrInvalidArgumentException::class, $e); + } + } + + #[Test] + public function itShouldIsolateCacheInstances(): void + { + $a = new InMemoryCache(); + $b = new InMemoryCache(); + + $a->set('secret', 'one'); + + self::assertFalse($b->has('secret')); + self::assertSame('one', $a->get('secret')); + } + + #[Test] + public function itShouldAllowSubclassesToIntrospectTheStore(): void + { + // Asserts the protected $store hook is usable from subclasses (test doubles, + // integrations). The fixture ArrayCache relies on this. + $double = new class extends InMemoryCache { + public function hasStoreKey(string $key): bool + { + return array_key_exists($key, $this->store); + } + }; + + $double->set('k', 'v'); + + self::assertTrue($double->hasStoreKey('k')); + } +} diff --git a/tests/unit/ResolverTest.php b/tests/unit/ResolverTest.php index 736c1e0..88f4642 100644 --- a/tests/unit/ResolverTest.php +++ b/tests/unit/ResolverTest.php @@ -16,13 +16,20 @@ use ReflectionClass; use ReflectionFunction; use ReflectionMethod; +use Respect\Parameter\InMemoryCache; use Respect\Parameter\ParameterResolver; use Respect\Parameter\Resolver; +use Respect\Parameter\Test\Fixtures\ArrayCache; use Respect\Parameter\Test\Fixtures\ArrayContainer; +use Respect\Parameter\Test\Fixtures\ConsumerWithExpensiveDefault; +use Respect\Parameter\Test\Fixtures\ExpensiveDefaultService; use Respect\Parameter\Test\Fixtures\SampleService; use Respect\Parameter\Test\Fixtures\ServiceConsumer; +use Respect\Parameter\Test\Fixtures\TwoRequiredConsumer; use Respect\Parameter\Test\Fixtures\VariadicConsumer; +use function md5; + #[CoversClass(Resolver::class)] final class ResolverTest extends TestCase { @@ -207,6 +214,147 @@ public function itShouldResolveNamedArgumentsReadyToSplat(): void self::assertSame(7, $consumer->number); } + #[Test] + public function itShouldInjectFromContainerWhenPositionalDoesNotMatchEvenWithCountMatch(): void + { + $service = new SampleService(); + $resolver = new Resolver(new ArrayContainer([SampleService::class => $service])); + + // count(args) === count(params) === 2, all params required, no variadic. + // The naive count-only fast path would return ['hello', 'world'] here, + // but the resolver must inject $service from the container and shift + // 'hello' onto $value (dropping 'world'). + $args = $resolver->resolve($this->constructorOf(TwoRequiredConsumer::class), ['hello', 'world']); + + self::assertSame([$service, 'hello'], $args); + } + + #[Test] + public function itShouldPassThroughWhenAllPositionalsAlignWithContainerResolvableType(): void + { + $service = new SampleService(); + $resolver = new Resolver(new ArrayContainer([SampleService::class => $service])); + + // Same shape as the previous test, but the positional already matches + // the parameter type — the sound fast path fires and returns the + // arguments unchanged, skipping introspection and container lookup. + $args = $resolver->resolve($this->constructorOf(TwoRequiredConsumer::class), [$service, 'hello']); + + self::assertSame([$service, 'hello'], $args); + } + + #[Test] + public function itShouldShareSpecAcrossDistinctReflectionsOfSameCallable(): void + { + $cache = new ArrayCache(); + $resolver = new Resolver( + new ArrayContainer([SampleService::class => new SampleService()]), + $cache, + ); + + // Two brand-new ReflectionMethod instances of the same constructor. + // Without a stable cache key, each would build and store its own spec + // (WeakMap keyed on object identity misses here). With the PSR-16 + // cache keyed on FQCN::method, the second resolve() must hit. + $r1 = $this->constructorOf(ServiceConsumer::class); + $r2 = $this->constructorOf(ServiceConsumer::class); + + self::assertNotSame($r1, $r2); + + $resolver->resolve($r1, ['hi']); + $resolver->resolve($r2, ['hi']); + + self::assertSame(1, $cache->sets, 'spec built and stored exactly once'); + self::assertSame(1, $cache->hits, 'second resolve() served from cache'); + self::assertSame(1, $cache->misses, 'first resolve() missed the cache'); + } + + #[Test] + public function itShouldBypassCacheWhenNoCacheSupplied(): void + { + // Default ctor: no cache. Resolver still works; spec is rebuilt per call. + $resolver = new Resolver(new ArrayContainer([SampleService::class => new SampleService()])); + + $args1 = $resolver->resolve($this->constructorOf(ServiceConsumer::class), ['hi']); + $args2 = $resolver->resolve($this->constructorOf(ServiceConsumer::class), ['hi']); + + self::assertSame($args1, $args2); + } + + #[Test] + public function itShouldMemoizeSpecAcrossReflectionsUsingBundledInMemoryCache(): void + { + // The bundled InMemoryCache (no external dependency) must work as a + // real spec cache: same callable, two fresh ReflectionMethod instances, + // spec is built once and served from cache on the second call. + $cache = new InMemoryCache(); + $resolver = new Resolver( + new ArrayContainer([SampleService::class => new SampleService()]), + $cache, + ); + + $r1 = $this->constructorOf(ServiceConsumer::class); + $r2 = $this->constructorOf(ServiceConsumer::class); + self::assertNotSame($r1, $r2); + + $args1 = $resolver->resolve($r1, ['hi']); + $args2 = $resolver->resolve($r2, ['hi']); + + self::assertSame($args1, $args2); + // Second call must be a cache hit: spec key present in the bundled cache. + self::assertTrue( + $cache->has( + 'respect.parameter.spec.' . md5('Respect\Parameter\Test\Fixtures\ServiceConsumer::__construct'), + ), + ); + } + + #[Test] + public function itShouldNotEagerlyConstructObjectDefaultsWhenSpecIsBuilt(): void + { + // PHP 8.1+ `new ExpensiveDefaultService()` as a parameter default must + // NOT run while the spec is built or cached — only when the default + // branch is actually taken in resolve(). Here we supply both positionals + // so the default branch never fires; if buildSpec() eagerly evaluated + // the default (the old behavior), an instance would still be constructed + // during spec build/cache and this test would fail. + ExpensiveDefaultService::$instances = 0; + $cache = new ArrayCache(); + $resolver = new Resolver(new ArrayContainer(), $cache); + $explicit = new ExpensiveDefaultService(); + $pre = ExpensiveDefaultService::$instances; + + $resolver->resolve( + $this->constructorOf(ConsumerWithExpensiveDefault::class), + ['hi', $explicit], + ); + + self::assertSame( + $pre, + ExpensiveDefaultService::$instances, + 'object default not eagerly constructed during spec build/cache', + ); + self::assertSame(1, $cache->sets, 'spec was built and stored once'); + } + + #[Test] + public function itShouldConstructObjectDefaultLazilyOnlyWhenDefaultBranchIsTaken(): void + { + ExpensiveDefaultService::$instances = 0; + $resolver = new Resolver(new ArrayContainer(), new ArrayCache()); + + // Supply no $service → the default branch fires → the object default + // is constructed exactly once for this call. + $args = $resolver->resolve($this->constructorOf(ConsumerWithExpensiveDefault::class), ['hi']); + + self::assertSame( + 1, + ExpensiveDefaultService::$instances, + 'object default constructed exactly once when default branch taken', + ); + self::assertInstanceOf(ExpensiveDefaultService::class, $args[1]); + } + /** @param class-string $class */ private function constructorOf(string $class): ReflectionMethod {