From 4bd4c4b5f1273cab399315d83eb8b9d5c4f1ae5d Mon Sep 17 00:00:00 2001 From: Jesse Leite Date: Thu, 9 Sep 2021 17:47:53 -0400 Subject: [PATCH 1/9] Add custom bindable user classes. --- src/Auth/Eloquent/UserRepository.php | 17 ++++++++++++----- src/Auth/UserRepository.php | 5 +++++ src/Fieldtypes/Users.php | 5 +++++ src/Providers/AuthServiceProvider.php | 10 +++++++++- src/Stache/Repositories/UserRepository.php | 12 +++++++----- 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/Auth/Eloquent/UserRepository.php b/src/Auth/Eloquent/UserRepository.php index 9910d5fb11f..e89b1072d76 100644 --- a/src/Auth/Eloquent/UserRepository.php +++ b/src/Auth/Eloquent/UserRepository.php @@ -19,11 +19,6 @@ public function __construct($config) $this->config = $config; } - public function make(): UserContract - { - return (new User)->model(new $this->config['model']); - } - public function all(): UserCollection { $users = $this->model('all')->keyBy('id')->map(function ($model) { @@ -106,4 +101,16 @@ public function fromUser($user): ?UserContract return null; } + + public static function bindings(): array + { + return [ + UserContract::class => User::class, + ]; + } + + public function make(): UserContract + { + return parent::make()->model(new $this->config['model']); + } } diff --git a/src/Auth/UserRepository.php b/src/Auth/UserRepository.php index 4d471b4218d..04d126abbbf 100644 --- a/src/Auth/UserRepository.php +++ b/src/Auth/UserRepository.php @@ -18,6 +18,11 @@ public function create() return app(UserFactory::class); } + public function make(): User + { + return app(User::class); + } + public function current(): ?User { if (! $user = auth()->user()) { diff --git a/src/Fieldtypes/Users.php b/src/Fieldtypes/Users.php index dab3f192650..0f4f2cee11e 100644 --- a/src/Fieldtypes/Users.php +++ b/src/Fieldtypes/Users.php @@ -117,6 +117,11 @@ protected function augmentValue($value) return User::find($value); } + protected function shallowAugmentValue($value) + { + return $value->toShallowAugmentedCollection(); + } + protected function getCreateItemUrl() { return cp_route('users.create'); diff --git a/src/Providers/AuthServiceProvider.php b/src/Providers/AuthServiceProvider.php index 82205540429..f2ac288a63a 100755 --- a/src/Providers/AuthServiceProvider.php +++ b/src/Providers/AuthServiceProvider.php @@ -43,7 +43,15 @@ public function register() }); $this->app->singleton(UserRepository::class, function ($app) { - return $app[UserRepositoryManager::class]->repository(); + $repository = $app[UserRepositoryManager::class]->repository(); + + foreach ($repository::bindings() as $abstract => $concrete) { + if (! $this->app->bound($abstract)) { + $this->app->bind($abstract, $concrete); + } + } + + return $repository; }); $this->app->singleton(RoleRepository::class, function ($app) { diff --git a/src/Stache/Repositories/UserRepository.php b/src/Stache/Repositories/UserRepository.php index 80263486875..f253277e675 100644 --- a/src/Stache/Repositories/UserRepository.php +++ b/src/Stache/Repositories/UserRepository.php @@ -26,11 +26,6 @@ public function __construct(Stache $stache, array $config = []) $this->config = $config; } - public function make(): User - { - return new FileUser; - } - public function all(): UserCollection { return $this->query()->get(); @@ -73,4 +68,11 @@ public function fromUser($user): ?User return null; } + + public static function bindings(): array + { + return [ + User::class => FileUser::class, + ]; + } } From c672a3dd128d848f073d5c19dcf05a02c06a1fda Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 14:17:30 -0400 Subject: [PATCH 2/9] Use make so the bound class gets used --- src/Auth/Eloquent/User.php | 1 + src/Auth/Eloquent/UserQueryBuilder.php | 3 ++- src/Auth/Eloquent/UserRepository.php | 17 +++-------------- 3 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/Auth/Eloquent/User.php b/src/Auth/Eloquent/User.php index acdbe671fb8..b9de6d121bc 100644 --- a/src/Auth/Eloquent/User.php +++ b/src/Auth/Eloquent/User.php @@ -20,6 +20,7 @@ class User extends BaseUser protected $roles; protected $groups; + /** @deprecated */ public static function fromModel(Model $model) { return tap(new static, function ($user) use ($model) { diff --git a/src/Auth/Eloquent/UserQueryBuilder.php b/src/Auth/Eloquent/UserQueryBuilder.php index 311b21f504e..c8879ad04cf 100644 --- a/src/Auth/Eloquent/UserQueryBuilder.php +++ b/src/Auth/Eloquent/UserQueryBuilder.php @@ -3,6 +3,7 @@ namespace Statamic\Auth\Eloquent; use Statamic\Auth\UserCollection; +use Statamic\Facades\User; use Statamic\Query\EloquentQueryBuilder; class UserQueryBuilder extends EloquentQueryBuilder @@ -10,7 +11,7 @@ class UserQueryBuilder extends EloquentQueryBuilder protected function transform($items, $columns = ['*']) { return UserCollection::make($items)->map(function ($model) { - return User::fromModel($model); + return User::make()->model($model); })->each->selectedQueryColumns($columns); } } diff --git a/src/Auth/Eloquent/UserRepository.php b/src/Auth/Eloquent/UserRepository.php index e89b1072d76..ec9b5b70877 100644 --- a/src/Auth/Eloquent/UserRepository.php +++ b/src/Auth/Eloquent/UserRepository.php @@ -22,7 +22,7 @@ public function __construct($config) public function all(): UserCollection { $users = $this->model('all')->keyBy('id')->map(function ($model) { - return $this->makeUser($model); + return $this->make()->model($model); }); return UserCollection::make($users); @@ -32,7 +32,7 @@ public function find($id): ?UserContract { return Blink::once("eloquent-user-find-{$id}", function () use ($id) { if ($model = $this->model('find', $id)) { - return $this->makeUser($model); + return $this->make()->model($model); } return null; @@ -45,7 +45,7 @@ public function findByEmail(string $email): ?UserContract return null; } - return $this->makeUser($model); + return $this->make()->model($model); } public function model($method, ...$args) @@ -55,17 +55,6 @@ public function model($method, ...$args) return call_user_func_array([$model, $method], $args); } - /** - * Convert an Eloquent User model to a Statamic User instance. - * - * @param Model $model - * @return User - */ - private function makeUser(Model $model) - { - return User::fromModel($model); - } - public function query() { return new UserQueryBuilder($this->model('query')); From 7beb2cdd84c9ff3a9d18b4dcbc1a0099ca0ec75f Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 14:53:28 -0400 Subject: [PATCH 3/9] test --- tests/Auth/EloquentUserRepositoryTest.php | 65 ++++++++++++++++++++ tests/Auth/StacheUserRepositoryTest.php | 32 ++++++++++ tests/Auth/UserRepositoryTests.php | 75 +++++++++++++++++++++++ 3 files changed, 172 insertions(+) create mode 100644 tests/Auth/EloquentUserRepositoryTest.php create mode 100644 tests/Auth/StacheUserRepositoryTest.php create mode 100644 tests/Auth/UserRepositoryTests.php diff --git a/tests/Auth/EloquentUserRepositoryTest.php b/tests/Auth/EloquentUserRepositoryTest.php new file mode 100644 index 00000000000..53a5639934f --- /dev/null +++ b/tests/Auth/EloquentUserRepositoryTest.php @@ -0,0 +1,65 @@ +set('statamic.users.repository', 'eloquent'); + $app['config']->set('auth.providers', [ + 'users' => [ + 'driver' => 'eloquent', + 'model' => ActualUserModel::class, + ], + ]); + + // Just so we can override saveToDatabase() + app()->bind(\Statamic\Auth\Eloquent\User::class, ActualEloquentUser::class); + } + + protected function defineDatabaseMigrations() + { + $this->loadLaravelMigrations(); + } + + public function userClass() + { + return ActualEloquentUser::class; + } + + public function fakeUserClass() + { + return FakeEloquentUser::class; + } +} + +class ActualEloquentUser extends \Statamic\Auth\Eloquent\User +{ + public function saveToDatabase() + { + $this->model()->save(); + + // dont save roles/groups + } +} + +class FakeEloquentUser extends ActualEloquentUser +{ + public function initials() + { + return 'FAKEINITIALS'; + } +} + +class ActualUserModel extends \Illuminate\Database\Eloquent\Model +{ + protected $table = 'users'; +} diff --git a/tests/Auth/StacheUserRepositoryTest.php b/tests/Auth/StacheUserRepositoryTest.php new file mode 100644 index 00000000000..33a763c636c --- /dev/null +++ b/tests/Auth/StacheUserRepositoryTest.php @@ -0,0 +1,32 @@ +assertInstanceOf($this->userClass(), User::make()); + } + + /** @test **/ + public function it_overrides_the_class() + { + app()->bind(\Statamic\Contracts\Auth\User::class, $this->fakeUserClass()); + + $this->assertInstanceOf($this->fakeUserClass(), $user = User::make()); + $this->assertEquals('FAKEINITIALS', $user->initials()); + } + + /** @test */ + public function it_gets_all_users() + { + User::make()->email('foo@bar.com')->data(['name' => 'foo', 'password' => 'foo'])->save(); + $this->assertEveryItemIsInstanceOf($this->userClass(), User::all()); + } + + /** @test */ + public function it_gets_all_users_with_overridden_classes() + { + app()->bind(\Statamic\Contracts\Auth\User::class, $this->fakeUserClass()); + + User::make()->email('foo@bar.com')->data(['name' => 'foo', 'password' => 'foo'])->save(); + $this->assertEveryItemIsInstanceOf($this->fakeUserClass(), User::all()); + } + + /** @test */ + public function it_gets_user_by_id() + { + User::make()->id(1)->email('foo@bar.com')->data(['name' => 'foo', 'password' => 'foo'])->save(); + $this->assertInstanceOf($this->userClass(), User::find(1)); + } + + /** @test */ + public function it_gets_user_by_id_with_overridden_classes() + { + app()->bind(\Statamic\Contracts\Auth\User::class, $this->fakeUserClass()); + + User::make()->id(1)->email('foo@bar.com')->data(['name' => 'foo', 'password' => 'foo'])->save(); + $this->assertInstanceOf($this->fakeUserClass(), User::find(1)); + } + + /** @test */ + public function it_gets_user_by_email() + { + User::make()->email('foo@bar.com')->data(['name' => 'foo', 'password' => 'foo'])->save(); + $this->assertInstanceOf($this->userClass(), User::findByEmail('foo@bar.com')); + } + + /** @test */ + public function it_gets_user_by_email_with_overridden_classes() + { + app()->bind(\Statamic\Contracts\Auth\User::class, $this->fakeUserClass()); + + User::make()->email('foo@bar.com')->data(['name' => 'foo', 'password' => 'foo'])->save(); + $this->assertInstanceOf($this->fakeUserClass(), User::findByEmail('foo@bar.com')); + } +} From 4b76dad3d5989d36a5be7c3168271d9ec973b7c9 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 15:39:45 -0400 Subject: [PATCH 4/9] skip --- tests/Auth/EloquentUserRepositoryTest.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/Auth/EloquentUserRepositoryTest.php b/tests/Auth/EloquentUserRepositoryTest.php index 53a5639934f..fafdd2930c9 100644 --- a/tests/Auth/EloquentUserRepositoryTest.php +++ b/tests/Auth/EloquentUserRepositoryTest.php @@ -9,6 +9,16 @@ class EloquentUserRepositoryTest extends TestCase { use UserRepositoryTests; + public function setUp(): void + { + parent::setUp(); + + if (version_compare($this->app->version(), '7.0', '<')) { + // honestly just a pain to support this in earlier versions of laravel/testbench + $this->markTestSkipped('Needs newer testbench'); + } + } + protected function getEnvironmentSetUp($app) { parent::getEnvironmentSetUp($app); From c85f1954ca84d093d5fb20126712c5330ff547c9 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 15:43:23 -0400 Subject: [PATCH 5/9] undo --- tests/Auth/EloquentUserRepositoryTest.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/Auth/EloquentUserRepositoryTest.php b/tests/Auth/EloquentUserRepositoryTest.php index fafdd2930c9..53a5639934f 100644 --- a/tests/Auth/EloquentUserRepositoryTest.php +++ b/tests/Auth/EloquentUserRepositoryTest.php @@ -9,16 +9,6 @@ class EloquentUserRepositoryTest extends TestCase { use UserRepositoryTests; - public function setUp(): void - { - parent::setUp(); - - if (version_compare($this->app->version(), '7.0', '<')) { - // honestly just a pain to support this in earlier versions of laravel/testbench - $this->markTestSkipped('Needs newer testbench'); - } - } - protected function getEnvironmentSetUp($app) { parent::getEnvironmentSetUp($app); From 8079d1c72e4efd0f65947a30b4c38e45db9be2d5 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 16:22:54 -0400 Subject: [PATCH 6/9] Check for dev packages --- src/Console/Composer/Lock.php | 5 ++++- tests/Fakes/Composer/Package/PackToTheFuture.php | 7 ++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Console/Composer/Lock.php b/src/Console/Composer/Lock.php index 37b720419ae..8fb00670a8a 100644 --- a/src/Console/Composer/Lock.php +++ b/src/Console/Composer/Lock.php @@ -99,7 +99,10 @@ public function getInstalledVersion(string $package) { $this->ensureExists(); - $installed = collect(json_decode($this->files->get($this->path))->packages) + $lock = json_decode($this->files->get($this->path)); + + $installed = collect($lock->packages) + ->merge($lock->{'packages-dev'}) ->keyBy('name') ->get($package); diff --git a/tests/Fakes/Composer/Package/PackToTheFuture.php b/tests/Fakes/Composer/Package/PackToTheFuture.php index f03ec7a5be9..1cae9330495 100644 --- a/tests/Fakes/Composer/Package/PackToTheFuture.php +++ b/tests/Fakes/Composer/Package/PackToTheFuture.php @@ -78,9 +78,8 @@ public static function generateComposerJson(string $package, string $version, ar */ public static function generateComposerLock(string $package, string $version, $path = null, $dev = false) { - $packagesKey = $dev - ? 'packages-dev' - : 'packages'; + $packagesKey = $dev ? 'packages-dev' : 'packages'; + $nonFavouritePackagesKey = $dev ? 'packages' : 'packages-dev'; $content = [ $packagesKey => [ @@ -89,6 +88,7 @@ public static function generateComposerLock(string $package, string $version, $p 'version' => $version, ], ], + $nonFavouritePackagesKey => [], ]; file_put_contents( @@ -117,6 +117,7 @@ public static function generateComposerLockForMultiple($packages, $path = null) $content = [ 'packages' => $packages, + 'packages-dev' => [], ]; file_put_contents( From 3df004b5fa993239e8e0ea05b6f684608de8e12b Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 16:23:07 -0400 Subject: [PATCH 7/9] Skip test on lower versions of testbench --- tests/Auth/EloquentUserRepositoryTest.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/Auth/EloquentUserRepositoryTest.php b/tests/Auth/EloquentUserRepositoryTest.php index 53a5639934f..aee669150c3 100644 --- a/tests/Auth/EloquentUserRepositoryTest.php +++ b/tests/Auth/EloquentUserRepositoryTest.php @@ -2,6 +2,7 @@ namespace Tests\Auth; +use Illuminate\Foundation\Testing\RefreshDatabase; use Tests\TestCase; /** @group user-repo */ @@ -9,6 +10,17 @@ class EloquentUserRepositoryTest extends TestCase { use UserRepositoryTests; + public function setUp(): void + { + parent::setup(); + + $testbench = (new \Statamic\Console\Processes\Composer(__DIR__.'/../../'))->installedVersion('orchestra/testbench-core'); + + if (version_compare($testbench, '6.7.0', '<')) { + $this->markTestSkipped('Need defineDatabaseMigrations method only introduced in 6.7.0'); + } + } + protected function getEnvironmentSetUp($app) { parent::getEnvironmentSetUp($app); From 1a70ede482fabb4d3cf103bfb72641a318bac2f6 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 20:23:27 +0000 Subject: [PATCH 8/9] Apply fixes from StyleCI [ci skip] [skip ci] --- tests/Auth/EloquentUserRepositoryTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Auth/EloquentUserRepositoryTest.php b/tests/Auth/EloquentUserRepositoryTest.php index aee669150c3..e197945586e 100644 --- a/tests/Auth/EloquentUserRepositoryTest.php +++ b/tests/Auth/EloquentUserRepositoryTest.php @@ -2,7 +2,6 @@ namespace Tests\Auth; -use Illuminate\Foundation\Testing\RefreshDatabase; use Tests\TestCase; /** @group user-repo */ From 24129e27b7a5c5ad127aca9148bd2ed5ecebd71f Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Tue, 28 Sep 2021 16:28:46 -0400 Subject: [PATCH 9/9] trigger ci