Sync: prevent infinite recursion during entity hydration

- Add `SyncEntityRecursionException`
- Add `ISyncContext::pushWithRecursionCheck()` and use it instead of
  `ISyncContext::push()` where necessary to prevent recursive deferral
  of entities and relationships
- Add `ISyncContext::maybeThrowRecursionException()` and call it before
  requesting entities from providers
- In `GetSyncEntities`, apply `DeferralPolicy::DO_NOT_RESOLVE` in
  addition to `HydrationPolicy::SUPPRESS` when `--shallow` is given
- Fix issue where deferred relationships are created to hydrate entities
  with no identifier
- Update return types and PHPDoc annotations to resolve method
  compatibility issues reported by PHPStan

Author: lkrms

lk-util/Command/Generate/Concept/GenerateCommand.php

@@ -200,7 +200,7 @@ abstract class GenerateCommand extends Command
     protected string $InputClassType;
 
     /**
-     * @var Introspector<object,Provider,Entity,ProviderContext>
+     * @var Introspector<object,Provider,Entity,ProviderContext<Provider,Entity>>
      */
     protected Introspector $InputIntrospector;
 

src/Util/Concept/Builder.php

@@ -40,7 +40,7 @@ protected static function getTerminators(): array
     protected ContainerInterface $Container;
 
     /**
-     * @var Introspector<object,Provider,Entity,ProviderContext>
+     * @var Introspector<object,Provider,Entity,ProviderContext<Provider,Entity>>
      */
     private Introspector $Introspector;
 

src/Util/Concept/Entity.php

@@ -8,6 +8,6 @@
 /**
  * Base class for entities
  *
- * @implements IProviderEntity<Provider,ProviderContext>
+ * @implements IProviderEntity<Provider,ProviderContext<Provider,self>>
  */
 abstract class Entity implements IProviderEntity {}

src/Util/Concept/Provider.php

@@ -4,14 +4,15 @@
 
 use Lkrms\Container\ContainerInterface;
 use Lkrms\Contract\IProvider;
+use Lkrms\Contract\IProviderContext;
 use Lkrms\Support\Date\DateFormatterInterface;
 use Lkrms\Support\ProviderContext;
 use Salient\Core\Exception\MethodNotImplementedException;
 
 /**
  * Base class for providers
  *
- * @implements IProvider<ProviderContext>
+ * @implements IProvider<ProviderContext<static,Entity>>
  */
 abstract class Provider implements IProvider
 {
@@ -39,11 +40,9 @@ abstract protected function getDateFormatter(): DateFormatterInterface;
     /**
      * @inheritDoc
      */
-    public function getContext(?ContainerInterface $container = null): ProviderContext
+    public function getContext(?ContainerInterface $container = null): IProviderContext
     {
-        if (!$container) {
-            $container = $this->App;
-        }
+        $container ??= $this->App;
 
         return $container->get(ProviderContext::class, [$this]);
     }
@@ -81,17 +80,15 @@ final public function container(): ContainerInterface
      */
     final public function dateFormatter(): DateFormatterInterface
     {
-        return $this->DateFormatter
-            ?? ($this->DateFormatter = $this->getDateFormatter());
+        return $this->DateFormatter ??= $this->getDateFormatter();
     }
 
     /**
-     * Get the date formatter cached by dateFormatter(), or null if it hasn't
-     * been cached
+     * Check if the date formatter returned by dateFormatter() has been cached
      */
-    final protected function getCachedDateFormatter(): ?DateFormatterInterface
+    final protected function hasDateFormatter(): bool
     {
-        return $this->DateFormatter ?? null;
+        return isset($this->DateFormatter);
     }
 
     /**

src/Util/Support/Introspector.php

@@ -107,7 +107,7 @@ class Introspector
      * @template T of object
      *
      * @param class-string<T> $service
-     * @return static<T,Provider,Entity,ProviderContext>
+     * @return static<T,Provider,Entity,ProviderContext<Provider,Entity>>
      */
     public static function getService(ContainerInterface $container, string $service)
     {
@@ -126,7 +126,7 @@ public static function getService(ContainerInterface $container, string $service
      * @template T of object
      *
      * @param class-string<T> $class
-     * @return static<T,Provider,Entity,ProviderContext>
+     * @return static<T,Provider,Entity,ProviderContext<Provider,Entity>>
      */
     public static function get(string $class)
     {

src/Util/Support/ProviderContext.php

@@ -14,9 +14,13 @@
 use Salient\Core\Utility\Str;
 
 /**
- * The context within which an entity is instantiated by a provider
+ * The context within which entities of a given type are instantiated by a
+ * provider
  *
- * @implements IProviderContext<IProvider<static>,IProvidable<IProvider<static>,static>>
+ * @template TProvider of IProvider
+ * @template TEntity of IProvidable
+ *
+ * @implements IProviderContext<TProvider,TEntity>
  */
 class ProviderContext implements IProviderContext
 {
@@ -25,12 +29,12 @@ class ProviderContext implements IProviderContext
     protected ContainerInterface $Container;
 
     /**
-     * @var IProvider<static>
+     * @var TProvider
      */
     protected IProvider $Provider;
 
     /**
-     * @var array<IProvidable<IProvider<static>,static>>
+     * @var TEntity[]
      */
     protected array $Stack = [];
 
@@ -40,7 +44,7 @@ class ProviderContext implements IProviderContext
     protected array $Values = [];
 
     /**
-     * @var (IProvidable<IProvider<static>,static>&ITreeable)|null
+     * @var (TEntity&ITreeable)|null
      */
     protected ?ITreeable $Parent = null;
 
@@ -52,7 +56,7 @@ class ProviderContext implements IProviderContext
     /**
      * Creates a new ProviderContext object
      *
-     * @param IProvider<static> $provider
+     * @param TProvider $provider
      */
     public function __construct(
         ContainerInterface $container,

src/Util/Sync/Command/GetSyncEntities.php

@@ -6,6 +6,7 @@
 use Lkrms\Cli\Exception\CliInvalidArgumentsException;
 use Lkrms\Cli\CliOption;
 use Lkrms\Facade\Console;
+use Lkrms\Sync\Catalog\DeferralPolicy;
 use Lkrms\Sync\Catalog\HydrationPolicy;
 use Lkrms\Sync\Contract\ISyncEntity;
 use Lkrms\Sync\Contract\ISyncProvider;
@@ -84,7 +85,7 @@ protected function getOptionList(): array
                 ->bindTo($this->Filter),
             CliOption::build()
                 ->long('shallow')
-                ->description('Do not hydrate entity relationships')
+                ->description('Do not resolve entity relationships')
                 ->bindTo($this->Shallow),
             CliOption::build()
                 ->long('include-canonical-id')
@@ -148,7 +149,9 @@ protected function run(string ...$args)
 
         $context = $provider->getContext();
         if ($this->Shallow || $this->Csv) {
-            $context = $context->withHydrationPolicy(HydrationPolicy::SUPPRESS);
+            $context = $context
+                ->withDeferralPolicy(DeferralPolicy::DO_NOT_RESOLVE)
+                ->withHydrationPolicy(HydrationPolicy::SUPPRESS);
         }
 
         $result = $this->EntityId !== null

src/Util/Sync/Concept/SyncDefinition.php

@@ -325,12 +325,13 @@ final public function getSyncOperationClosure($operation): ?Closure
 
         // If a method has been declared for this operation, use it, even if
         // it's not in $this->Operations
-        if ($closure =
-                $this->ProviderIntrospector->getDeclaredSyncOperationClosure(
-                    $operation,
-                    $this->EntityIntrospector,
-                    $this->Provider
-                )) {
+        $closure = $this->ProviderIntrospector->getDeclaredSyncOperationClosure(
+            $operation,
+            $this->EntityIntrospector,
+            $this->Provider
+        );
+
+        if ($closure) {
             return $this->Closures[$operation] =
                 fn(ISyncContext $ctx, ...$args) =>
                     $closure(
@@ -344,10 +345,9 @@ final public function getSyncOperationClosure($operation): ?Closure
                 ($closure = $this->getSyncOperationClosure(OP::READ_LIST))) {
             return $this->Closures[$operation] =
                 function (ISyncContext $ctx, $id, ...$args) use ($closure) {
-                    $entity =
-                        $this
-                            ->getFluentIterator($closure($ctx, ...$args))
-                            ->nextWithValue('Id', $id);
+                    $entity = $this
+                        ->getFluentIterator($closure($ctx, ...$args))
+                        ->nextWithValue('Id', $id);
                     if ($entity === null) {
                         throw new SyncEntityNotFoundException($this->Provider, $this->Entity, $id);
                     }

src/Util/Sync/Concept/SyncProvider.php

@@ -83,7 +83,7 @@ public function __construct(ContainerInterface $app, SyncStore $store)
     /**
      * @inheritDoc
      */
-    public function getContext(?ContainerInterface $container = null): SyncContext
+    public function getContext(?ContainerInterface $container = null): ISyncContext
     {
         if (!$container) {
             $container = $this->App;
@@ -249,10 +249,14 @@ final public static function getServices(): array
      */
     final public function with(string $entity, $context = null): SyncEntityProvider
     {
-        /** @var ContainerInterface */
-        $container = $context instanceof ISyncContext
-            ? $context->container()
-            : ($context ?: $this->App);
+        if ($context instanceof ISyncContext) {
+            $context->maybeThrowRecursionException();
+            $container = $context->container();
+        } else {
+            /** @var ContainerInterface */
+            $container = $context ?? $this->App;
+        }
+
         $container = $container->inContextOf(static::class);
 
         $context = $context instanceof ISyncContext

src/Util/Sync/Contract/ISyncContext.php

@@ -9,10 +9,11 @@
 use Lkrms\Sync\Catalog\HydrationPolicy;
 use Lkrms\Sync\Catalog\SyncOperation;
 use Lkrms\Sync\Concept\SyncProvider;
+use Lkrms\Sync\Exception\SyncEntityRecursionException;
 use Lkrms\Sync\Exception\SyncInvalidFilterException;
 
 /**
- * The context within which a sync entity is instantiated by a provider
+ * The context within which sync entities are instantiated by a provider
  *
  * @extends IProviderContext<ISyncProvider,ISyncEntity>
  */
@@ -62,7 +63,7 @@ interface ISyncContext extends IProviderContext
      * @param SyncOperation::* $operation
      * @param mixed ...$args Sync operation arguments, NOT including the
      * {@see ISyncContext} argument.
-     * @return $this
+     * @return static
      */
     public function withArgs($operation, ...$args);
 
@@ -76,14 +77,14 @@ public function withArgs($operation, ...$args);
      * @see ISyncContext::applyFilterPolicy()
      *
      * @param (callable(ISyncContext, ?bool &$returnEmpty, array{}|null &$empty): void)|null $callback
-     * @return $this
+     * @return static
      */
     public function withFilterPolicyCallback(?callable $callback);
 
     /**
      * Reject entities from the local entity store
      *
-     * @return $this
+     * @return static
      */
     public function online();
 
@@ -93,7 +94,7 @@ public function online();
      * An exception is thrown if the local entity store is unable to satisfy
      * subsequent entity requests.
      *
-     * @return $this
+     * @return static
      */
     public function offline();
 
@@ -103,15 +104,15 @@ public function offline();
      *
      * This is the default behaviour.
      *
-     * @return $this
+     * @return static
      */
     public function offlineFirst();
 
     /**
      * Apply the given deferral policy to the context
      *
      * @param DeferralPolicy::* $policy
-     * @return $this
+     * @return static
      */
     public function withDeferralPolicy($policy);
 
@@ -123,14 +124,33 @@ public function withDeferralPolicy($policy);
      * change to an entity and its subclasses.
      * @param array<int<1,max>>|int<1,max>|null $depth Limit the scope of the
      * change to entities at a given `$depth` from the current context.
-     * @return $this
+     * @return static
      */
     public function withHydrationPolicy(
         int $policy,
         ?string $entity = null,
         $depth = null
     );
 
+    /**
+     * Push the entity propagating the context onto the stack after checking if
+     * it is already present
+     *
+     * {@see ISyncContext::maybeThrowRecursionException()} fails with an
+     * exception if `$entity` is already in the stack.
+     *
+     * @return static
+     */
+    public function pushWithRecursionCheck(ISyncEntity $entity);
+
+    /**
+     * Throw an exception if recursion is detected
+     *
+     * @throws SyncEntityRecursionException if
+     * {@see ISyncContext::pushWithRecursionCheck()} detected recursion.
+     */
+    public function maybeThrowRecursionException(): void;
+
     /**
      * Run the unclaimed filter policy callback
      *
@@ -163,7 +183,7 @@ public function withHydrationPolicy(
      *
      * @param array{}|null $empty
      */
-    public function applyFilterPolicy(?bool &$returnEmpty, &$empty): void;
+    public function applyFilterPolicy(?bool &$returnEmpty, ?array &$empty): void;
 
     /**
      * Get the filters passed to the context via optional sync operation
@@ -196,7 +216,7 @@ public function getFilter(string $key, bool $orValue = true);
      *
      * Unlike other {@see ISyncContext} methods,
      * {@see ISyncContext::claimFilter()} modifies the object it is called on
-     * instead of returning a modified clone.
+     * instead of returning a modified instance.
      *
      * If `$orValue` is `true` and a value for `$key` has been applied to the
      * context via {@see IProviderContext::withValue()}, it is returned if there