Skip to content

Commit

Permalink
chore: A bit of tidying-up and fixing (#53)
Browse files Browse the repository at this point in the history
* feat: Add helpers for retrieving resolvers, tenancies and tenant providers

* refactor: Move resolver tests to match namespacing of main package

* test(resolvers): Add tests for custom parameter names

* test(resolvers): Update tests to test for default and custom parameter names as well as default and custom parameter patterns

* chore: Add code coverage ignore to code that should theoretically never be reached

* fix: Fix issue where possible lazy-loading of tenant relation occurs

* test(tenanted-models): Add test for when hydration is strictly disabled

* test(resolvers): Add test for session identity resolver and session override compatibility issues

* refactor: Remove unused parameter from NoTenantFound exception

* chore: Doc comment updates
  • Loading branch information
ollieread authored Nov 4, 2024
1 parent 541fe49 commit 0950104
Show file tree
Hide file tree
Showing 14 changed files with 249 additions and 28 deletions.
10 changes: 5 additions & 5 deletions src/Concerns/FindsIdentityInRouteParameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public function getPattern(): ?string
*/
public function hasPattern(): bool
{
return $this->pattern !== null;
return $this->getPattern() !== null;
}

/**
Expand All @@ -155,7 +155,7 @@ public function getParameter(): string
/**
* Get the route parameter pattern mapping
*
* This method returns an array mappings the route parameter name to its
* This method returns an array mapping the route parameter name to its
* pattern, for use in route definitions.
*
* @template TenantClass of \Sprout\Contracts\Tenant
Expand All @@ -167,7 +167,7 @@ public function getParameter(): string
protected function getParameterPatternMapping(Tenancy $tenancy): array
{
if (! $this->hasPattern()) {
return [];
return []; // @codeCoverageIgnore
}

return [
Expand All @@ -191,7 +191,7 @@ protected function getParameterPatternMapping(Tenancy $tenancy): array
*/
protected function applyParameterPatternMapping(RouteRegistrar $registrar, Tenancy $tenancy): RouteRegistrar
{
if ($this->hasPattern()) {
if (! $this->hasPattern()) {
return $registrar;
}

Expand Down Expand Up @@ -219,7 +219,7 @@ public function resolveFromRoute(Route $route, Tenancy $tenancy, Request $reques
return $identifier;
}

return null;
return null; // @codeCoverageIgnore
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public function retrieved(Model $model): void
}

if (! TenancyOptions::shouldHydrateTenantRelation($tenancy)) {
return;
return; // @codeCoverageIgnore
}

/**
Expand Down
9 changes: 2 additions & 7 deletions src/Exceptions/NoTenantFound.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,11 @@ final class NoTenantFound extends SproutException
*
* @param string $resolver
* @param string $tenancy
* @param string|null $identity
*
* @return self
*/
public static function make(string $resolver, string $tenancy, ?string $identity = null): self
public static function make(string $resolver, string $tenancy): self
{
return new self(
$identity
? 'No valid tenant [' . $tenancy . '] found for \'' . $identity . '\', resolved via [' . $resolver . ']'
: 'No valid tenant [' . $tenancy . '] found [' . $resolver . ']'
);
return new self('No valid tenant [' . $tenancy . '] found [' . $resolver . ']');
}
}
2 changes: 2 additions & 0 deletions src/Exceptions/TenancyMissing.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
* expected/required.
*
* @package Core
*
* @codeCoverageIgnore
*/
final class TenancyMissing extends SproutException
{
Expand Down
3 changes: 0 additions & 3 deletions src/Overrides/CacheOverride.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
use Illuminate\Cache\NullStore;
use Illuminate\Cache\RedisStore;
use Illuminate\Contracts\Foundation\Application;
use RuntimeException;
use Sprout\Contracts\BootableServiceOverride;
use Sprout\Contracts\Tenancy;
use Sprout\Contracts\Tenant;
Expand All @@ -28,8 +27,6 @@
* cache service.
*
* @package Overrides
*
* @codeCoverageIgnore
*/
final class CacheOverride implements BootableServiceOverride
{
Expand Down
3 changes: 0 additions & 3 deletions src/Overrides/SessionOverride.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Illuminate\Contracts\Foundation\Application;
use Illuminate\Session\FileSessionHandler;
use Illuminate\Session\SessionManager;
use RuntimeException;
use Sprout\Concerns\OverridesCookieSettings;
use Sprout\Contracts\BootableServiceOverride;
use Sprout\Contracts\Tenancy;
Expand All @@ -27,8 +26,6 @@
* session service.
*
* @package Overrides
*
* @codeCoverageIgnore
*/
final class SessionOverride implements BootableServiceOverride
{
Expand Down
49 changes: 49 additions & 0 deletions src/helpers.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

namespace Sprout;

use Sprout\Contracts\IdentityResolver;
use Sprout\Contracts\Tenancy;
use Sprout\Contracts\TenantProvider;

/**
* Get the core sprout class
*
Expand All @@ -11,3 +15,48 @@ function sprout(): Sprout
{
return app(Sprout::class);
}

/**
* Get an identity resolver
*
* @param string|null $name
*
* @return \Sprout\Contracts\IdentityResolver
*
* @throws \Illuminate\Contracts\Container\BindingResolutionException
* @throws \Sprout\Exceptions\MisconfigurationException
*/
function resolver(?string $name = null): IdentityResolver
{
return sprout()->resolvers()->get($name);
}

/**
* Get a tenancy
*
* @param string|null $name
*
* @return \Sprout\Contracts\Tenancy<*>
*
* @throws \Illuminate\Contracts\Container\BindingResolutionException
* @throws \Sprout\Exceptions\MisconfigurationException
*/
function tenancy(?string $name = null): Tenancy
{
return sprout()->tenancies()->get($name);
}

/**
* Get a tenant provider
*
* @param string|null $name
*
* @return \Sprout\Contracts\TenantProvider<*>
*
* @throws \Illuminate\Contracts\Container\BindingResolutionException
* @throws \Sprout\Exceptions\MisconfigurationException
*/
function provider(?string $name = null): TenantProvider
{
return sprout()->providers()->get($name);
}
2 changes: 1 addition & 1 deletion tests/Database/Eloquent/BelongsToManyTenantsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public function automaticallyPopulateTheTenantRelationWhenHydrating(): void

$this->assertTrue($child->exists);
$this->assertTrue($child->relationLoaded('tenants'));
$this->assertNotNull($child->tenants->first(fn (Model $model) => $model->is($tenant)));
$this->assertNotNull($child->getRelation('tenants')->first(fn (Model $model) => $model->is($tenant)));
}

#[Test]
Expand Down
20 changes: 19 additions & 1 deletion tests/Database/Eloquent/BelongsToTenantTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,25 @@ public function automaticallyPopulateTheTenantRelationWhenHydrating(): void

$this->assertTrue($child->exists);
$this->assertTrue($child->relationLoaded('tenant'));
$this->assertTrue($child->tenant->is($tenant));
$this->assertNotNull($child->getRelation('tenant'));
$this->assertTrue($child->getRelation('tenant')->is($tenant));
}

#[Test]
public function doNotHydrateWhenHydrateTenantRelationIsMissing(): void
{
/** @var \Sprout\Contracts\Tenancy $tenancy */
$tenancy = app(TenancyManager::class)->get();
$tenancy->removeOption(TenancyOptions::hydrateTenantRelation());

$tenant = TenantModel::factory()->create();

$tenancy->setTenant($tenant);

$child = TenantChild::query()->find(TenantChild::factory()->create()->getKey());

$this->assertTrue($child->exists);
$this->assertFalse($child->relationLoaded('tenant'));
}

#[Test]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php
declare(strict_types=1);

namespace Sprout\Tests\Resolvers;
namespace Http\Resolvers;

use Illuminate\Config\Repository;
use Illuminate\Foundation\Testing\RefreshDatabase;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<?php
declare(strict_types=1);

namespace Sprout\Tests\Resolvers;
namespace Http\Resolvers;

use Illuminate\Config\Repository;
use Illuminate\Foundation\Testing\RefreshDatabase;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
<?php
declare(strict_types=1);

namespace Sprout\Tests\Resolvers;
namespace Http\Resolvers;

use Illuminate\Config\Repository;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Routing\Router;
use Orchestra\Testbench\Attributes\DefineEnvironment;
use Orchestra\Testbench\Concerns\WithWorkbench;
use Orchestra\Testbench\TestCase;
use PHPUnit\Framework\Attributes\Test;
use Sprout\Attributes\CurrentTenant;
use Sprout\Contracts\Tenant;
use Workbench\App\Models\TenantModel;
use function Sprout\resolver;

class PathResolverTest extends TestCase
{
Expand All @@ -27,7 +29,35 @@ protected function defineEnvironment($app): void
});
}

protected function defineRoutes($router)
protected function withManualParameterName($app): void
{
tap($app['config'], static function (Repository $config) {
$config->set('multitenancy.resolvers.path.parameter', 'custom-parameter');
});
}

protected function withParameterPatternName($app): void
{
tap($app['config'], static function (Repository $config) {
$config->set('multitenancy.resolvers.path.pattern', '.*');
});
}

protected function withoutManualParameterName($app): void
{
tap($app['config'], static function (Repository $config) {
$config->set('multitenancy.resolvers.path.parameter', null);
});
}

protected function withoutParameterPattern($app): void
{
tap($app['config'], static function (Repository $config) {
$config->set('multitenancy.resolvers.path.pattern', null);
});
}

protected function defineRoutes($router): void
{
$router->get('/', function () {
return 'no';
Expand Down Expand Up @@ -81,4 +111,40 @@ public function throwsExceptionForInvalidTenantWithoutParameter(): void

$result->assertInternalServerError();
}

#[Test, DefineEnvironment('withoutParameterPattern')]
public function hasNoParameterPatternByDefault(): void
{
/** @var \Sprout\Http\Resolvers\SubdomainIdentityResolver $resolver */
$resolver = resolver('path');

$this->assertNull($resolver->getPattern());
}

#[Test, DefineEnvironment('withoutManualParameterName')]
public function hasDefaultParameterNameByDefault(): void
{
/** @var \Sprout\Http\Resolvers\SubdomainIdentityResolver $resolver */
$resolver = resolver('path');

$this->assertSame('{tenancy}_{resolver}', $resolver->getParameter());
}

#[Test, DefineEnvironment('withManualParameterName')]
public function allowsForCustomParameterName(): void
{
/** @var \Sprout\Http\Resolvers\PathIdentityResolver $resolver */
$resolver = resolver('path');

$this->assertSame('custom-parameter', $resolver->getParameter());
}

#[Test, DefineEnvironment('withParameterPatternName')]
public function allowsForCustomParameterPattern(): void
{
/** @var \Sprout\Http\Resolvers\PathIdentityResolver $resolver */
$resolver = resolver('path');

$this->assertSame('.*', $resolver->getPattern());
}
}
Original file line number Diff line number Diff line change
@@ -1,25 +1,28 @@
<?php
declare(strict_types=1);

namespace Sprout\Tests\Resolvers;
namespace Http\Resolvers;

use Illuminate\Config\Repository;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Routing\Router;
use Illuminate\Session\Middleware\StartSession;
use Orchestra\Testbench\Attributes\DefineEnvironment;
use Orchestra\Testbench\Concerns\WithWorkbench;
use Orchestra\Testbench\TestCase;
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\Attributes\Test;
use Sprout\Attributes\CurrentTenant;
use Sprout\Contracts\Tenant;
use Sprout\Exceptions\CompatibilityException;
use Sprout\Overrides\AuthOverride;
use Sprout\Overrides\CacheOverride;
use Sprout\Overrides\CookieOverride;
use Sprout\Overrides\JobOverride;
use Sprout\Overrides\SessionOverride;
use Sprout\Overrides\StorageOverride;
use Workbench\App\Models\TenantModel;
use function Sprout\sprout;

#[Group('resolvers'), Group('sessions')]
class SessionResolverTest extends TestCase
Expand Down Expand Up @@ -47,6 +50,20 @@ protected function defineEnvironment($app): void
});
}

protected function withSessionOverride($app):void
{
tap($app['config'], static function (Repository $config) {
$config->set('sprout.services', [
StorageOverride::class,
JobOverride::class,
CacheOverride::class,
AuthOverride::class,
CookieOverride::class,
SessionOverride::class,
]);
});
}

protected function defineRoutes($router): void
{
$router->middleware(StartSession::class)->group(function (Router $router) {
Expand Down Expand Up @@ -88,4 +105,19 @@ public function throwsExceptionWithoutHeader(): void

$result->assertInternalServerError();
}

#[Test]
public function throwsExceptionIfSessionOverrideIsEnabled(): void
{
sprout()->addOverride(new SessionOverride());
$tenant = TenantModel::factory()->createOne();

$result = $this->withSession(['multitenancy' => ['tenants' => $tenant->getTenantIdentifier()]])->get(route('session.route'));
$result->assertInternalServerError();
$this->assertInstanceOf(CompatibilityException::class, $result->exception);
$this->assertSame(
'Cannot use resolver [session] with override [' . SessionOverride::class . ']',
$result->exception->getMessage()
);
}
}
Loading

0 comments on commit 0950104

Please sign in to comment.