Simplify resolvers

src/Macros.php

@@ -10,7 +10,6 @@
 use Glhd\Gretel\Routing\RequestBreadcrumbs;
 use Glhd\Gretel\Routing\RouteBreadcrumb;
 use Illuminate\Routing\Route;
-use Illuminate\Support\Str;
 
 class Macros
 {
@@ -34,18 +33,13 @@ public static function breadcrumb(
 		$parent = null,
 		Closure $relation = null
 	): Route {
-		if (!$route->getName()) {
+		if (!$name = $route->getName()) {
 			throw new UnnamedRouteException();
 		}
 
-		$name = $route->getName();
-		$parameters = $route->parameterNames();
-
-		$title = TitleResolver::make($title, $parameters);
-		$url = UrlResolver::makeForRoute($name, $parameters);
-
-		$parent = static::resolveParent($registry, $name, $parent);
-		$parent = ParentResolver::makeWithRelation($parent, $parameters, $relation);
+		$title = TitleResolver::make($title);
+		$parent = ParentResolver::make($parent, $name, $relation);
+		$url = UrlResolver::make($name, $route->parameterNames());
 
 		$registry->register(new RouteBreadcrumb($name, $title, $parent, $url));
 
@@ -56,24 +50,4 @@ public static function breadcrumbs(Registry $registry, Route $route): RequestBre
 	{
 		return new RequestBreadcrumbs($registry, $route);
 	}
-
-	protected static function resolveParent(Registry $registry, string $name, $parent)
-	{
-		if ($parent instanceof Closure) {
-			return static function(...$args) use ($name, $parent) {
-				$result = $parent(...$args);
-				return Macros::resolveParent(app(Registry::class), $name, $result);
-			};
-		}
-
-		if (!is_string($parent)) {
-			return $parent;
-		}
-
-		if (0 === strpos($parent, '.')) {
-			$parent = Str::beforeLast($name, '.').$parent;
-		}
-
-		return $registry->getOrFail($parent);
-	}
 }

src/Resolvers/ParentResolver.php

@@ -2,84 +2,73 @@
 
 namespace Glhd\Gretel\Resolvers;
 
+use Arr;
 use Closure;
 use Glhd\Gretel\Exceptions\UnmatchedRouteException;
 use Glhd\Gretel\Registry;
 use Glhd\Gretel\Routing\RouteBreadcrumb;
 use Illuminate\Http\Request;
-use Illuminate\Support\Arr;
+use Illuminate\Support\Str;
 use RuntimeException;
 use Symfony\Component\HttpFoundation\Request as SymfonyRequest;
 use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
 
 class ParentResolver extends Resolver
 {
-	public static function makeWithRelation($value, array $parameters = [], Closure $relation = null): Resolver
+	public static function make($value, string $name, ?Closure $relation = null): Resolver
 	{
-		if ($relation) {
-			if ($value instanceof RouteBreadcrumb) {
-				$parent = clone $value;
-			} else {
-				$parent = $value;
-			}
-
-			$relation = static::optimizeBinding($relation);
-
-			if ($parent instanceof Closure) {
-				$parent = static::optimizeBinding($parent);
-			}
-
-			$value = static function($parameters) use ($parent, $relation) {
-				$parameters = array_values($parameters);
-
-				if ($parent instanceof Closure) {
-					$parent = clone $parent(...$parameters);
-				}
-
-				return $parent->setParameters(Arr::wrap($relation(...$parameters)));
-			};
-		} elseif ($value instanceof Closure) {
-			// If we're been passed a closure, we need to pass the parameters
-			// in as individual arguments.
-			$original = $value;
-			$value = static function($parameters) use ($original) {
-				return call_user_func_array($original, array_values($parameters));
-			};
-		}
+		$value = static::wrapClosure($value);
+		$relation = static::wrapNullableClosure($relation);
+
+		$callback = static function() use ($value, $name, $relation) {
+			return [$value, $name, $relation];
+		};
 
-		return parent::make($value, $parameters);
+		return new static($callback);
 	}
 
 	public function resolve(array $parameters, Registry $registry)
 	{
-		$result = parent::resolve($parameters, $registry);
+		[$callback, $name, $relation] = parent::resolve($parameters, $registry);
+		$result = $callback($parameters);
 
-		if (is_string($result)) {
-			// If we get back a URL, we'll try to resolve the parent via the Router
-			if (filter_var($result, FILTER_VALIDATE_URL)) {
-				return $this->findParentByUrl($result, $registry);
-			}
-
-			// If we get back a route name, we'll load it from the registry and pass
-			// on any custom parameters that were provided
-			if ($registry->has($result)) {
-				$parent = clone $registry->getOrFail($result);
-				return $parent->setParameters($parameters);
-			}
+		if (null === $result) {
+			return null;
+		}
+
+		if ($relation) {
+			$parameters = Arr::wrap($relation($parameters));
+		}
+
+		// Handle parent shorthand
+		if (is_string($result) && 0 === strpos($result, '.')) {
+			$result = Str::beforeLast($name, '.').$result;
+		}
+
+		// If we get back a route name, we'll load it from the registry and pass
+		// on any custom parameters that were provided
+		if (is_string($result) && $registry->has($result)) {
+			$result = $registry->getOrFail($result);
+		}
+
+		// If we get back a URL, we'll try to resolve the parent via the Router
+		// This may not last in the API — use at your own risk
+		if (is_string($result) && filter_var($result, FILTER_VALIDATE_URL)) {
+			return $this->findParentByUrl($result, $registry);
 		}
 
 		if (!($result instanceof RouteBreadcrumb)) {
 			throw new RuntimeException('Unable to resolve parent breadcrumb.');
 		}
 
+		if (!empty($parameters)) {
+			$result = clone $result;
+			$result->setParameters($parameters);
+		}
+
 		return $result;
 	}
 
-	protected function transformParameters(array $parameters, Registry $registry): array
-	{
-		return [$parameters];
-	}
-
 	protected function findParentByUrl(string $url, Registry $registry): RouteBreadcrumb
 	{
 		$router = app('router');

src/Resolvers/Resolver.php

@@ -19,26 +19,38 @@ class Resolver
 
 	protected ?string $serialized = null;
 
-	public static function make($value, array $parameters = []): self
+	protected static function wrap($value): self
+	{
+		return $value instanceof self
+			? $value
+			: new static(static::wrapClosure($value));
+	}
+
+	protected static function wrapClosure($value): Closure
 	{
-		// If the value is already a resolver, no need to do anything
-		if ($value instanceof self) {
-			return $value;
-		}
-
-		// If value is a closure, we'll use late static binding
 		if ($value instanceof Closure) {
-			return new static($value, $parameters);
+			$value = static::optimizeBinding($value);
+			return static function($parameters) use ($value) {
+				return $value(...array_values($parameters));
+			};
 		}
 
-		// Otherwise, we'll instantiate a plain/base resolver instance
-		return new self(fn() => $value, $parameters);
+		return static function() use ($value) {
+			return $value;
+		};
+	}
+
+	protected static function wrapNullableClosure($value): ?Closure
+	{
+		return null === $value
+			? $value
+			: static::wrapClosure($value);
 	}
 
 	/**
 	 * @var \Closure|string $callback
 	 */
-	public function __construct($callback, array $parameters)
+	public function __construct($callback)
 	{
 		if ($callback instanceof Closure) {
 			$this->callback = static::optimizeBinding($callback);
@@ -47,16 +59,11 @@ public function __construct($callback, array $parameters)
 		} else {
 			throw new InvalidArgumentException('Resolver callbacks must be a Closure or a serialized closure.');
 		}
-
-		$this->parameters = $parameters;
 	}
 
-	/**
-	 * @return \Glhd\Gretel\Routing\RouteBreadcrumb|string
-	 */
 	public function resolve(array $parameters, Registry $registry)
 	{
-		return call_user_func_array($this->getClosure(), $this->transformParameters($parameters, $registry));
+		return call_user_func($this->getClosure(), $parameters, $registry);
 	}
 
 	public function getClosure(): Closure
@@ -72,7 +79,7 @@ public function getClosure(): Closure
 		return $this->callback;
 	}
 
-	public function exportForSerialization(): array
+	public function getSerializedClosure(): string
 	{
 		if (null === $this->serialized) {
 			$callback = $this->callback;
@@ -81,12 +88,7 @@ public function exportForSerialization(): array
 			$this->serialized = serialize(new SerializableClosure($callback));
 		}
 
-		return [$this->parameters, $this->serialized];
-	}
-
-	protected function transformParameters(array $parameters, Registry $registry): array
-	{
-		return array_values($parameters);
+		return $this->serialized;
 	}
 
 	protected function isSerializedClosure($value): bool

src/Resolvers/TitleResolver.php

@@ -4,4 +4,8 @@
 
 class TitleResolver extends Resolver
 {
+	public static function make($value): Resolver
+	{
+		return static::wrap($value);
+	}
 }

src/Resolvers/UrlResolver.php

@@ -2,25 +2,20 @@
 
 namespace Glhd\Gretel\Resolvers;
 
-use Glhd\Gretel\Registry;
 use Illuminate\Support\Arr;
 
 class UrlResolver extends Resolver
 {
-	public static function makeForRoute(string $name, array $parameters): self
+	public static function make(string $name, array $parameter_names): self
 	{
-		$callback = function(array $route_parameters) use ($name, $parameters) {
+		$callback = function(array $route_parameters) use ($name, $parameter_names) {
 			$keys = Arr::isAssoc($route_parameters)
-				? $parameters
-				: array_keys($parameters);
+				? $parameter_names
+				: array_keys($parameter_names);
+
 			return route($name, Arr::only($route_parameters, $keys));
 		};
 
-		return static::make($callback, $parameters);
-	}
-
-	protected function transformParameters(array $parameters, Registry $registry): array
-	{
-		return [$parameters];
+		return new self($callback);
 	}
 }

src/Support/BindsClosures.php

@@ -6,9 +6,9 @@
 
 trait BindsClosures
 {
-	protected static function optimizeBinding(Closure $closure): Closure
+	protected static function optimizeBinding(?Closure $closure): ?Closure
 	{
-		return config('gretel.static_closures')
+		return null !== $closure && config('gretel.static_closures')
 			? $closure->bindTo(null)
 			: $closure;
 	}

src/Support/Cache.php

@@ -97,16 +97,9 @@ protected function exportBreadcrumb(RouteBreadcrumb $breadcrumb): string
 
 	protected function exportResolver(Resolver $resolver): string
 	{
-		[$parameters, $callback] = $resolver->exportForSerialization();
-
-		$parameters = empty($parameters)
-			? '[]'
-			: var_export($parameters, true);
-
-		$callback = var_export($callback, true);
-
 		$fqcn = get_class($resolver);
+		$callback = var_export($resolver->getSerializedClosure(), true);
 
-		return "new \\{$fqcn}({$callback}, {$parameters})";
+		return "new \\{$fqcn}({$callback})";
 	}
 }

tests/RouteMacroTest.php

@@ -23,6 +23,8 @@ protected function setUp(): void
 		$this->note = Note::factory()->create(['user_id' => $this->user->id]);
 
 		$this->artisan('breadcrumbs:clear');
+
+		$this->withoutExceptionHandling(); // FIXME
 	}
 
 	/** @dataProvider cachingProvider */
@@ -197,6 +199,24 @@ public function test_dynamic_parent(bool $cache): void
 		);
 	}
 
+	/** @dataProvider cachingProvider */
+	public function test_breadcrumbs_can_be_registered_out_of_order(bool $cache): void
+	{
+		Route::get('/users/create', $this->action())
+			->name('users.create')
+			->breadcrumb('Add a User', 'users.index');
+
+		Route::get('/users', $this->action())
+			->name('users.index')
+			->breadcrumb('Users');
+
+		$this->setUpCache($cache);
+
+		$this->get('/users/create');
+
+		$this->assertActiveBreadcrumbs(['Users', '/users'], ['Add a User', '/users/create']);
+	}
+
 	public function cachingProvider(): array
 	{
 		return [