From 3a64794f9fef3fac2e3a69bdbc6eb796f2e25b1b Mon Sep 17 00:00:00 2001 From: Brian Faust Date: Fri, 5 May 2023 05:48:48 +0300 Subject: [PATCH 1/2] Add support for generating controller policies https://laravel.com/docs/10.x/authorization#authorizing-actions-using-policies --- config/blueprint.php | 3 + src/Generators/ControllerGenerator.php | 42 ++++- src/Generators/PolicyGenerator.php | 65 +++++++ src/Lexers/ControllerLexer.php | 27 ++- src/Models/Controller.php | 14 ++ src/Models/Policy.php | 137 +++++++++++++++ src/Tree.php | 5 + stubs/controller.authorize-resource.stub | 4 + stubs/controller.policy-with-model.stub | 1 + stubs/controller.policy-without-model.stub | 1 + stubs/policy.class.stub | 12 ++ stubs/policy.method.create.stub | 7 + stubs/policy.method.delete.stub | 7 + stubs/policy.method.forceDelete.stub | 7 + stubs/policy.method.restore.stub | 7 + stubs/policy.method.update.stub | 7 + stubs/policy.method.view.stub | 7 + stubs/policy.method.viewAny.stub | 7 + .../Generators/ControllerGeneratorTest.php | 98 +++++++++++ .../Generators/PolicyGeneratorTest.php | 162 ++++++++++++++++++ tests/Feature/Lexers/ControllerLexerTest.php | 56 +++++- .../controllers/with-all-policies.php | 76 ++++++++ .../controllers/with-authorize-resource.php | 65 +++++++ .../controllers/with-some-policies.php | 64 +++++++ .../drafts/controller-with-all-policies.yaml | 5 + .../controller-with-authorize-resource.yaml | 5 + .../drafts/controller-with-some-policies.yaml | 5 + .../drafts/policy-with-all-policies.yaml | 5 + .../policy-with-authorize-resource.yaml | 5 + .../drafts/policy-with-some-policies.yaml | 5 + tests/fixtures/policies/with-all-policies.php | 68 ++++++++ .../policies/with-authorize-resource.php | 68 ++++++++ .../fixtures/policies/with-some-policies.php | 28 +++ 33 files changed, 1069 insertions(+), 6 deletions(-) create mode 100644 src/Generators/PolicyGenerator.php create mode 100644 src/Models/Policy.php create mode 100644 stubs/controller.authorize-resource.stub create mode 100644 stubs/controller.policy-with-model.stub create mode 100644 stubs/controller.policy-without-model.stub create mode 100644 stubs/policy.class.stub create mode 100644 stubs/policy.method.create.stub create mode 100644 stubs/policy.method.delete.stub create mode 100644 stubs/policy.method.forceDelete.stub create mode 100644 stubs/policy.method.restore.stub create mode 100644 stubs/policy.method.update.stub create mode 100644 stubs/policy.method.view.stub create mode 100644 stubs/policy.method.viewAny.stub create mode 100644 tests/Feature/Generators/PolicyGeneratorTest.php create mode 100644 tests/fixtures/controllers/with-all-policies.php create mode 100644 tests/fixtures/controllers/with-authorize-resource.php create mode 100644 tests/fixtures/controllers/with-some-policies.php create mode 100644 tests/fixtures/drafts/controller-with-all-policies.yaml create mode 100644 tests/fixtures/drafts/controller-with-authorize-resource.yaml create mode 100644 tests/fixtures/drafts/controller-with-some-policies.yaml create mode 100644 tests/fixtures/drafts/policy-with-all-policies.yaml create mode 100644 tests/fixtures/drafts/policy-with-authorize-resource.yaml create mode 100644 tests/fixtures/drafts/policy-with-some-policies.yaml create mode 100644 tests/fixtures/policies/with-all-policies.php create mode 100644 tests/fixtures/policies/with-authorize-resource.php create mode 100644 tests/fixtures/policies/with-some-policies.php diff --git a/config/blueprint.php b/config/blueprint.php index e8cf6c79..25347f9b 100644 --- a/config/blueprint.php +++ b/config/blueprint.php @@ -30,6 +30,8 @@ 'controllers_namespace' => 'Http\\Controllers', + 'policy_namespace' => 'Policies', + /* |-------------------------------------------------------------------------- | Application Path @@ -141,6 +143,7 @@ 'notification' => \Blueprint\Generators\Statements\NotificationGenerator::class, 'resource' => \Blueprint\Generators\Statements\ResourceGenerator::class, 'view' => \Blueprint\Generators\Statements\ViewGenerator::class, + 'policy' => \Blueprint\Generators\PolicyGenerator::class, ], ]; diff --git a/src/Generators/ControllerGenerator.php b/src/Generators/ControllerGenerator.php index 3fb927fd..33b38fe2 100644 --- a/src/Generators/ControllerGenerator.php +++ b/src/Generators/ControllerGenerator.php @@ -6,6 +6,7 @@ use Blueprint\Concerns\HandlesTraits; use Blueprint\Contracts\Generator; use Blueprint\Models\Controller; +use Blueprint\Models\Policy; use Blueprint\Models\Statements\DispatchStatement; use Blueprint\Models\Statements\EloquentStatement; use Blueprint\Models\Statements\FireStatement; @@ -62,22 +63,55 @@ protected function buildMethods(Controller $controller) $methods = ''; + $controllerModelName = Str::singular($controller->prefix()); + + if ($controller->policy()?->authorizeResource()) { + $methods .= str_replace( + [ + '{{ modelClass }}', + '{{ modelVariable }}', + ], + [ + Str::studly($controllerModelName), + Str::camel($controllerModelName), + ], + $this->filesystem->stub('controller.authorize-resource.stub') + ); + } + foreach ($controller->methods() as $name => $statements) { $method = str_replace('{{ method }}', $name, $template); if (in_array($name, ['edit', 'update', 'show', 'destroy'])) { - $context = Str::singular($controller->prefix()); - $reference = $this->fullyQualifyModelReference($controller->namespace(), Str::camel($context)); - $variable = '$' . Str::camel($context); + $reference = $this->fullyQualifyModelReference($controller->namespace(), $controllerModelName); + $variable = '$' . Str::camel($controllerModelName); $search = '(Request $request'; - $method = str_replace($search, $search . ', ' . $context . ' ' . $variable, $method); + $method = str_replace($search, $search . ', ' . $controllerModelName . ' ' . $variable, $method); $this->addImport($controller, $reference); } $body = ''; $using_validation = false; + if ($controller->policy() && !$controller->policy()->authorizeResource()) { + if (in_array($name, $controller->policy()->methods())) { + $body .= str_replace( + [ + '{{ method }}', + '{{ modelClass }}', + '{{ modelVariable }}', + ], + [ + $name, + Str::studly($controllerModelName), + '$' . Str::camel($controllerModelName), + ], + $this->filesystem->stub(Policy::$resourceAbilityStubMap[$name]) + ) . PHP_EOL; + } + } + foreach ($statements as $statement) { if ($statement instanceof SendStatement) { $body .= self::INDENT . $statement->output() . PHP_EOL; diff --git a/src/Generators/PolicyGenerator.php b/src/Generators/PolicyGenerator.php new file mode 100644 index 00000000..8042c22b --- /dev/null +++ b/src/Generators/PolicyGenerator.php @@ -0,0 +1,65 @@ +tree = $tree; + + $stub = $this->filesystem->stub('policy.class.stub'); + + /** @var \Blueprint\Models\Policy $policy */ + foreach ($tree->policies() as $policy) { + $this->addImport($policy, $policy->fullyQualifiedModelClassName()); + + $path = $this->getPath($policy); + + $this->create($path, $this->populateStub($stub, $policy)); + } + + return $this->output; + } + + protected function populateStub(string $stub, Policy $policy) + { + $stub = str_replace('{{ namespace }}', $policy->fullyQualifiedNamespace(), $stub); + $stub = str_replace('{{ class }}', $policy->className(), $stub); + $stub = str_replace('{{ methods }}', $this->buildMethods($policy), $stub); + $stub = str_replace('{{ imports }}', $this->buildImports($policy), $stub); + + return $stub; + } + + protected function buildMethods(Policy $policy) + { + $methods = ''; + + foreach ($policy->methods() as $name) { + $methods .= str_replace( + [ + '{{ modelClass }}', + '{{ modelVariable }}', + ], + [ + Str::studly($policy->name()), + Str::camel($policy->name()), + ], + $this->filesystem->stub('policy.method.' . Policy::$resourceAbilityMap[$name] . '.stub'), + ) . PHP_EOL; + } + + return $methods; + } +} diff --git a/src/Lexers/ControllerLexer.php b/src/Lexers/ControllerLexer.php index 76c8c04a..4c97a90b 100644 --- a/src/Lexers/ControllerLexer.php +++ b/src/Lexers/ControllerLexer.php @@ -4,6 +4,8 @@ use Blueprint\Contracts\Lexer; use Blueprint\Models\Controller; +use Blueprint\Models\Policy; +use Illuminate\Support\Arr; use Illuminate\Support\Str; class ControllerLexer implements Lexer @@ -20,7 +22,10 @@ public function __construct(StatementLexer $statementLexer) public function analyze(array $tokens): array { - $registry = ['controllers' => []]; + $registry = [ + 'controllers' => [], + 'policies' => [], + ]; if (empty($tokens['controllers'])) { return $registry; @@ -50,6 +55,26 @@ public function analyze(array $tokens): array unset($definition['invokable']); } + if (isset($definition['meta'])) { + if (isset($definition['meta']['policies'])) { + $authorizeResource = Arr::get($definition, 'meta.policies', true); + + $policy = new Policy( + $controller->prefix(), + $authorizeResource === true + ? array_keys(Policy::$resourceAbilityMap) + : preg_split('/,([ \t]+)?/', $definition['meta']['policies']), + $authorizeResource === true, + ); + + $controller->policy($policy); + + $registry['policies'][] = $policy; + } + + unset($definition['meta']); + } + foreach ($definition as $method => $body) { $controller->addMethod($method, $this->statementLexer->analyze($body)); } diff --git a/src/Models/Controller.php b/src/Models/Controller.php index 79c472f1..c72f47b4 100644 --- a/src/Models/Controller.php +++ b/src/Models/Controller.php @@ -28,6 +28,11 @@ class Controller implements BlueprintModel */ private $methods = []; + /** + * @var Policy + */ + private $policy; + /** * @var bool */ @@ -91,6 +96,15 @@ public function addMethod(string $name, array $statements) $this->methods[$name] = $statements; } + public function policy(?Policy $policy = null): ?Policy + { + if ($policy) { + $this->policy = $policy; + } + + return $this->policy; + } + public function prefix() { if (Str::endsWith($this->name(), 'Controller')) { diff --git a/src/Models/Policy.php b/src/Models/Policy.php new file mode 100644 index 00000000..2eadaa97 --- /dev/null +++ b/src/Models/Policy.php @@ -0,0 +1,137 @@ + 'viewAny', + 'show' => 'view', + 'create' => 'create', + 'store' => 'create', + 'edit' => 'update', + 'update' => 'update', + 'destroy' => 'delete', + ]; + + /** @var array */ + public static $resourceAbilityStubMap = [ + 'index' => 'controller.policy-without-model.stub', + 'show' => 'controller.policy-without-model.stub', + 'create' => 'controller.policy-without-model.stub', + 'store' => 'controller.policy-without-model.stub', + 'edit' => 'controller.policy-with-model.stub', + 'update' => 'controller.policy-with-model.stub', + 'destroy' => 'controller.policy-with-model.stub', + ]; + + /** + * @var string + */ + private $name; + + /** + * @var string + */ + private $namespace; + + /** + * @var array + */ + private $methods; + + /** + * @var bool + */ + private $authorizeResource; + + /** + * Controller constructor. + */ + public function __construct(string $name, array $methods, bool $authorizeResource) + { + $this->name = class_basename($name); + $this->namespace = trim(implode('\\', array_slice(explode('\\', str_replace('/', '\\', $name)), 0, -1)), '\\'); + $this->methods = $methods; + $this->authorizeResource = $authorizeResource; + } + + public function name(): string + { + return $this->name; + } + + public function className(): string + { + return $this->name() . (Str::endsWith($this->name(), 'Policy') ? '' : 'Policy'); + } + + public function namespace() + { + if (empty($this->namespace)) { + return ''; + } + + return $this->namespace; + } + + public function fullyQualifiedNamespace() + { + $fqn = config('blueprint.namespace'); + + if (config('blueprint.policy_namespace')) { + $fqn .= '\\' . config('blueprint.policy_namespace'); + } + + if ($this->namespace) { + $fqn .= '\\' . $this->namespace; + } + + return $fqn; + } + + public function fullyQualifiedClassName() + { + return $this->fullyQualifiedNamespace() . '\\' . $this->className(); + } + + public function methods(): array + { + return $this->methods; + } + + public function authorizeResource(): bool + { + return $this->authorizeResource; + } + + public function fullyQualifiedModelClassName() + { + $fqn = config('blueprint.namespace'); + + if (config('blueprint.models_namespace')) { + $fqn .= '\\' . config('blueprint.models_namespace'); + } + + if ($this->namespace) { + $fqn .= '\\' . $this->namespace; + } + + return $fqn . '\\' . $this->name; + } +} diff --git a/src/Tree.php b/src/Tree.php index ef7ddd9d..1b5b4068 100644 --- a/src/Tree.php +++ b/src/Tree.php @@ -30,6 +30,11 @@ public function models() return $this->tree['models']; } + public function policies() + { + return $this->tree['policies']; + } + public function seeders() { return $this->tree['seeders']; diff --git a/stubs/controller.authorize-resource.stub b/stubs/controller.authorize-resource.stub new file mode 100644 index 00000000..cb19b4d9 --- /dev/null +++ b/stubs/controller.authorize-resource.stub @@ -0,0 +1,4 @@ + public function __construct() + { + $this->authorizeResource({{ modelClass }}::class, '{{ modelVariable }}'); + } diff --git a/stubs/controller.policy-with-model.stub b/stubs/controller.policy-with-model.stub new file mode 100644 index 00000000..9381e7e8 --- /dev/null +++ b/stubs/controller.policy-with-model.stub @@ -0,0 +1 @@ + $this->authorize('{{ method }}', {{ modelVariable }}); diff --git a/stubs/controller.policy-without-model.stub b/stubs/controller.policy-without-model.stub new file mode 100644 index 00000000..48f15788 --- /dev/null +++ b/stubs/controller.policy-without-model.stub @@ -0,0 +1 @@ + $this->authorize('{{ method }}', {{ modelClass }}::class); diff --git a/stubs/policy.class.stub b/stubs/policy.class.stub new file mode 100644 index 00000000..e3946714 --- /dev/null +++ b/stubs/policy.class.stub @@ -0,0 +1,12 @@ +assertEquals(['created' => ['src/path/Other/Http/UserController.php']], $this->subject->output($tree)); } + #[Test] + public function output_generates_controller_with_all_policies(): void + { + $definition = 'drafts/controller-with-all-policies.yaml'; + $path = 'app/Http/Controllers/PostController.php'; + $controller = 'controllers/with-all-policies.php'; + + $this->filesystem->expects('stub') + ->with('controller.class.stub') + ->andReturn($this->stub('controller.class.stub')); + $this->filesystem->expects('stub') + ->with('controller.method.stub') + ->andReturn($this->stub('controller.method.stub')); + $this->filesystem->expects('stub') + ->times(3) + ->with('controller.policy-with-model.stub') + ->andReturn($this->stub('controller.policy-with-model.stub')); + $this->filesystem->expects('stub') + ->times(4) + ->with('controller.policy-without-model.stub') + ->andReturn($this->stub('controller.policy-without-model.stub')); + + $this->filesystem->expects('exists') + ->with(dirname($path)) + ->andReturnTrue(); + $this->filesystem->expects('put') + ->with($path, $this->fixture($controller)); + + $tokens = $this->blueprint->parse($this->fixture($definition)); + $tree = $this->blueprint->analyze($tokens); + $this->assertEquals(['created' => [$path]], $this->subject->output($tree)); + } + + #[Test] + public function output_generates_controller_with_some_policies(): void + { + $definition = 'drafts/controller-with-some-policies.yaml'; + $path = 'app/Http/Controllers/PostController.php'; + $controller = 'controllers/with-some-policies.php'; + + $this->filesystem->expects('stub') + ->with('controller.class.stub') + ->andReturn($this->stub('controller.class.stub')); + $this->filesystem->expects('stub') + ->with('controller.method.stub') + ->andReturn($this->stub('controller.method.stub')); + $this->filesystem->expects('stub') + ->times(2) + ->with('controller.policy-without-model.stub') + ->andReturn($this->stub('controller.policy-without-model.stub')); + + $this->filesystem->expects('exists') + ->with(dirname($path)) + ->andReturnTrue(); + $this->filesystem->expects('put') + ->with($path, $this->fixture($controller)); + + $tokens = $this->blueprint->parse($this->fixture($definition)); + $tree = $this->blueprint->analyze($tokens); + $this->assertEquals(['created' => [$path]], $this->subject->output($tree)); + } + + #[Test] + public function output_generates_controller_with_authorize_resource(): void + { + $definition = 'drafts/controller-with-authorize-resource.yaml'; + $path = 'app/Http/Controllers/PostController.php'; + $controller = 'controllers/with-authorize-resource.php'; + + $this->filesystem->expects('stub') + ->with('controller.class.stub') + ->andReturn($this->stub('controller.class.stub')); + $this->filesystem->expects('stub') + ->with('controller.method.stub') + ->andReturn($this->stub('controller.method.stub')); + $this->filesystem->expects('stub') + ->with('controller.authorize-resource.stub') + ->andReturn($this->stub('controller.authorize-resource.stub')); + $this->filesystem->expects('stub') + ->times(0) + ->with('controller.policy-with-model.stub') + ->andReturn($this->stub('controller.policy-with-model.stub')); + $this->filesystem->expects('stub') + ->times(0) + ->with('controller.policy-without-model.stub') + ->andReturn($this->stub('controller.policy-without-model.stub')); + + $this->filesystem->expects('exists') + ->with(dirname($path)) + ->andReturnTrue(); + $this->filesystem->expects('put') + ->with($path, $this->fixture($controller)); + + $tokens = $this->blueprint->parse($this->fixture($definition)); + $tree = $this->blueprint->analyze($tokens); + $this->assertEquals(['created' => [$path]], $this->subject->output($tree)); + } + public static function controllerTreeDataProvider(): array { return [ diff --git a/tests/Feature/Generators/PolicyGeneratorTest.php b/tests/Feature/Generators/PolicyGeneratorTest.php new file mode 100644 index 00000000..36017158 --- /dev/null +++ b/tests/Feature/Generators/PolicyGeneratorTest.php @@ -0,0 +1,162 @@ +subject = new PolicyGenerator($this->filesystem); + + $this->blueprint = new Blueprint(); + $this->blueprint->registerLexer(new \Blueprint\Lexers\ModelLexer()); + $this->blueprint->registerLexer(new \Blueprint\Lexers\ControllerLexer(new StatementLexer())); + $this->blueprint->registerGenerator($this->subject); + } + + #[Test] + public function output_writes_nothing_for_empty_tree(): void + { + $this->filesystem->expects('stub') + ->with('policy.class.stub') + ->andReturn($this->stub('policy.class.stub')); + + $this->filesystem->shouldNotHaveReceived('put'); + + $this->assertEquals([], $this->subject->output(new Tree(['controllers' => [], 'policies' => []]))); + } + + #[Test] + public function output_generates_policies_for_controller_with_all_policies(): void + { + $definition = 'drafts/policy-with-all-policies.yaml'; + $path = 'app/Policies/PostPolicy.php'; + $policy = 'policies/with-all-policies.php'; + + $this->filesystem->expects('stub') + ->with('policy.class.stub') + ->andReturn($this->stub('policy.class.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.viewAny.stub') + ->andReturn($this->stub('policy.method.viewAny.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.view.stub') + ->andReturn($this->stub('policy.method.view.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.create.stub') + ->andReturn($this->stub('policy.method.create.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.create.stub') + ->andReturn($this->stub('policy.method.create.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.update.stub') + ->andReturn($this->stub('policy.method.update.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.update.stub') + ->andReturn($this->stub('policy.method.update.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.delete.stub') + ->andReturn($this->stub('policy.method.delete.stub')); + + $this->filesystem->expects('exists') + ->with(dirname($path)) + ->andReturnTrue(); + $this->filesystem->expects('put') + ->with($path, $this->fixture($policy)); + + $tokens = $this->blueprint->parse($this->fixture($definition)); + $tree = $this->blueprint->analyze($tokens); + + $this->assertEquals(['created' => [$path]], $this->subject->output($tree)); + } + + #[Test] + public function output_generates_policies_for_controller_with_some_policies(): void + { + $definition = 'drafts/policy-with-some-policies.yaml'; + $path = 'app/Policies/PostPolicy.php'; + $policy = 'policies/with-some-policies.php'; + + $this->filesystem->expects('stub') + ->with('policy.class.stub') + ->andReturn($this->stub('policy.class.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.viewAny.stub') + ->andReturn($this->stub('policy.method.viewAny.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.view.stub') + ->andReturn($this->stub('policy.method.view.stub')); + + $this->filesystem->expects('exists') + ->with(dirname($path)) + ->andReturnTrue(); + $this->filesystem->expects('put') + ->with($path, $this->fixture($policy)); + + $tokens = $this->blueprint->parse($this->fixture($definition)); + $tree = $this->blueprint->analyze($tokens); + + $this->assertEquals(['created' => [$path]], $this->subject->output($tree)); + } + + #[Test] + public function output_generates_policies_for_controller_with_authorize_resource(): void + { + $definition = 'drafts/policy-with-authorize-resource.yaml'; + $path = 'app/Policies/PostPolicy.php'; + $policy = 'policies/with-authorize-resource.php'; + + $this->filesystem->expects('stub') + ->with('policy.class.stub') + ->andReturn($this->stub('policy.class.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.viewAny.stub') + ->andReturn($this->stub('policy.method.viewAny.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.view.stub') + ->andReturn($this->stub('policy.method.view.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.create.stub') + ->andReturn($this->stub('policy.method.create.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.create.stub') + ->andReturn($this->stub('policy.method.create.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.update.stub') + ->andReturn($this->stub('policy.method.update.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.update.stub') + ->andReturn($this->stub('policy.method.update.stub')); + $this->filesystem->expects('stub') + ->with('policy.method.delete.stub') + ->andReturn($this->stub('policy.method.delete.stub')); + + $this->filesystem->expects('exists') + ->with(dirname($path)) + ->andReturnTrue(); + $this->filesystem->expects('put') + ->with($path, $this->fixture($policy)); + + $tokens = $this->blueprint->parse($this->fixture($definition)); + $tree = $this->blueprint->analyze($tokens); + + $this->assertEquals(['created' => [$path]], $this->subject->output($tree)); + } +} diff --git a/tests/Feature/Lexers/ControllerLexerTest.php b/tests/Feature/Lexers/ControllerLexerTest.php index f21dc092..6c40d3a3 100644 --- a/tests/Feature/Lexers/ControllerLexerTest.php +++ b/tests/Feature/Lexers/ControllerLexerTest.php @@ -4,6 +4,7 @@ use Blueprint\Lexers\ControllerLexer; use Blueprint\Lexers\StatementLexer; +use Blueprint\Models\Policy; use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; @@ -31,7 +32,10 @@ protected function setUp(): void #[Test] public function it_returns_nothing_without_controllers_token(): void { - $this->assertEquals(['controllers' => []], $this->subject->analyze([])); + $this->assertEquals([ + 'controllers' => [], + 'policies' => [], + ], $this->subject->analyze([])); } #[Test] @@ -432,4 +436,54 @@ public function it_returns_an_invokable_controller(): void $this->assertCount(1, $controller->methods()); $this->assertFalse($controller->isApiResource()); } + + #[Test] + public function it_returns_an_authorized_controller_with_all_policies(): void + { + $tokens = [ + 'controllers' => [ + 'Report' => [ + 'meta' => [ + 'policies' => true, + ], + ], + ], + ]; + + $this->statementLexer->shouldReceive('analyze'); + + $actual = $this->subject->analyze($tokens); + + $this->assertCount(1, $actual['controllers']); + $this->assertCount(1, $actual['policies']); + + $controller = $actual['controllers']['Report']; + $this->assertInstanceOf(Policy::class, $controller->policy()); + $this->assertEquals(array_keys(Policy::$resourceAbilityMap), $controller->policy()->methods()); + } + + #[Test] + public function it_returns_an_authorized_controller_with_specific_policies(): void + { + $tokens = [ + 'controllers' => [ + 'Report' => [ + 'meta' => [ + 'policies' => 'index,show', + ], + ], + ], + ]; + + $this->statementLexer->shouldReceive('analyze'); + + $actual = $this->subject->analyze($tokens); + + $this->assertCount(1, $actual['controllers']); + $this->assertCount(1, $actual['policies']); + + $controller = $actual['controllers']['Report']; + $this->assertInstanceOf(Policy::class, $controller->policy()); + $this->assertEquals(['index', 'show'], $controller->policy()->methods()); + } } diff --git a/tests/fixtures/controllers/with-all-policies.php b/tests/fixtures/controllers/with-all-policies.php new file mode 100644 index 00000000..65901098 --- /dev/null +++ b/tests/fixtures/controllers/with-all-policies.php @@ -0,0 +1,76 @@ +authorize('index', Post::class); + + $posts = Post::all(); + + return view('post.index', compact('posts')); + } + + public function create(Request $request): View + { + $this->authorize('create', Post::class); + + return view('post.create'); + } + + public function store(PostStoreRequest $request): RedirectResponse + { + $this->authorize('store', Post::class); + + + $post = Post::create($request->validated()); + + $request->session()->flash('post.id', $post->id); + + return redirect()->route('post.index'); + } + + public function show(Request $request, Post $post): View + { + $this->authorize('show', Post::class); + + return view('post.show', compact('post')); + } + + public function edit(Request $request, Post $post): View + { + $this->authorize('edit', $post); + + return view('post.edit', compact('post')); + } + + public function update(PostUpdateRequest $request, Post $post): RedirectResponse + { + $this->authorize('update', $post); + + + $post->update($request->validated()); + + $request->session()->flash('post.id', $post->id); + + return redirect()->route('post.index'); + } + + public function destroy(Request $request, Post $post): RedirectResponse + { + $this->authorize('destroy', $post); + + $post->delete(); + + return redirect()->route('post.index'); + } +} diff --git a/tests/fixtures/controllers/with-authorize-resource.php b/tests/fixtures/controllers/with-authorize-resource.php new file mode 100644 index 00000000..5cc99a49 --- /dev/null +++ b/tests/fixtures/controllers/with-authorize-resource.php @@ -0,0 +1,65 @@ +authorizeResource(Post::class, 'post'); + } + + public function index(Request $request): View + { + $posts = Post::all(); + + return view('post.index', compact('posts')); + } + + public function create(Request $request): View + { + return view('post.create'); + } + + public function store(PostStoreRequest $request): RedirectResponse + { + $post = Post::create($request->validated()); + + $request->session()->flash('post.id', $post->id); + + return redirect()->route('post.index'); + } + + public function show(Request $request, Post $post): View + { + return view('post.show', compact('post')); + } + + public function edit(Request $request, Post $post): View + { + return view('post.edit', compact('post')); + } + + public function update(PostUpdateRequest $request, Post $post): RedirectResponse + { + $post->update($request->validated()); + + $request->session()->flash('post.id', $post->id); + + return redirect()->route('post.index'); + } + + public function destroy(Request $request, Post $post): RedirectResponse + { + $post->delete(); + + return redirect()->route('post.index'); + } +} diff --git a/tests/fixtures/controllers/with-some-policies.php b/tests/fixtures/controllers/with-some-policies.php new file mode 100644 index 00000000..eb7b46b5 --- /dev/null +++ b/tests/fixtures/controllers/with-some-policies.php @@ -0,0 +1,64 @@ +authorize('index', Post::class); + + $posts = Post::all(); + + return view('post.index', compact('posts')); + } + + public function create(Request $request): View + { + return view('post.create'); + } + + public function store(PostStoreRequest $request): RedirectResponse + { + $post = Post::create($request->validated()); + + $request->session()->flash('post.id', $post->id); + + return redirect()->route('post.index'); + } + + public function show(Request $request, Post $post): View + { + $this->authorize('show', Post::class); + + return view('post.show', compact('post')); + } + + public function edit(Request $request, Post $post): View + { + return view('post.edit', compact('post')); + } + + public function update(PostUpdateRequest $request, Post $post): RedirectResponse + { + $post->update($request->validated()); + + $request->session()->flash('post.id', $post->id); + + return redirect()->route('post.index'); + } + + public function destroy(Request $request, Post $post): RedirectResponse + { + $post->delete(); + + return redirect()->route('post.index'); + } +} diff --git a/tests/fixtures/drafts/controller-with-all-policies.yaml b/tests/fixtures/drafts/controller-with-all-policies.yaml new file mode 100644 index 00000000..c067859c --- /dev/null +++ b/tests/fixtures/drafts/controller-with-all-policies.yaml @@ -0,0 +1,5 @@ +controllers: + Post: + resource + meta: + policies: index,create,store,show,edit,update,destroy diff --git a/tests/fixtures/drafts/controller-with-authorize-resource.yaml b/tests/fixtures/drafts/controller-with-authorize-resource.yaml new file mode 100644 index 00000000..e58fc6df --- /dev/null +++ b/tests/fixtures/drafts/controller-with-authorize-resource.yaml @@ -0,0 +1,5 @@ +controllers: + Post: + resource + meta: + policies: true diff --git a/tests/fixtures/drafts/controller-with-some-policies.yaml b/tests/fixtures/drafts/controller-with-some-policies.yaml new file mode 100644 index 00000000..0d3a0b50 --- /dev/null +++ b/tests/fixtures/drafts/controller-with-some-policies.yaml @@ -0,0 +1,5 @@ +controllers: + Post: + resource + meta: + policies: index,show diff --git a/tests/fixtures/drafts/policy-with-all-policies.yaml b/tests/fixtures/drafts/policy-with-all-policies.yaml new file mode 100644 index 00000000..c067859c --- /dev/null +++ b/tests/fixtures/drafts/policy-with-all-policies.yaml @@ -0,0 +1,5 @@ +controllers: + Post: + resource + meta: + policies: index,create,store,show,edit,update,destroy diff --git a/tests/fixtures/drafts/policy-with-authorize-resource.yaml b/tests/fixtures/drafts/policy-with-authorize-resource.yaml new file mode 100644 index 00000000..e58fc6df --- /dev/null +++ b/tests/fixtures/drafts/policy-with-authorize-resource.yaml @@ -0,0 +1,5 @@ +controllers: + Post: + resource + meta: + policies: true diff --git a/tests/fixtures/drafts/policy-with-some-policies.yaml b/tests/fixtures/drafts/policy-with-some-policies.yaml new file mode 100644 index 00000000..0d3a0b50 --- /dev/null +++ b/tests/fixtures/drafts/policy-with-some-policies.yaml @@ -0,0 +1,5 @@ +controllers: + Post: + resource + meta: + policies: index,show diff --git a/tests/fixtures/policies/with-all-policies.php b/tests/fixtures/policies/with-all-policies.php new file mode 100644 index 00000000..8aae21df --- /dev/null +++ b/tests/fixtures/policies/with-all-policies.php @@ -0,0 +1,68 @@ + Date: Sat, 6 May 2023 14:52:53 +0300 Subject: [PATCH 2/2] code review changes --- src/Generators/ControllerGenerator.php | 10 ++++++---- src/Generators/PolicyGenerator.php | 4 ++-- src/Lexers/ControllerLexer.php | 9 +++++++-- src/Models/Policy.php | 11 ---------- stubs/controller.policy-with-model.stub | 1 - stubs/controller.policy-without-model.stub | 1 - .../Generators/ControllerGeneratorTest.php | 20 ------------------- .../Generators/PolicyGeneratorTest.php | 12 ----------- tests/Feature/Lexers/ControllerLexerTest.php | 4 ++-- .../controllers/with-all-policies.php | 2 +- .../controllers/with-some-policies.php | 2 +- tests/fixtures/policies/with-all-policies.php | 20 +------------------ .../policies/with-authorize-resource.php | 20 +------------------ .../fixtures/policies/with-some-policies.php | 4 +--- 14 files changed, 22 insertions(+), 98 deletions(-) delete mode 100644 stubs/controller.policy-with-model.stub delete mode 100644 stubs/controller.policy-without-model.stub diff --git a/src/Generators/ControllerGenerator.php b/src/Generators/ControllerGenerator.php index 33b38fe2..5591e2b0 100644 --- a/src/Generators/ControllerGenerator.php +++ b/src/Generators/ControllerGenerator.php @@ -95,8 +95,8 @@ protected function buildMethods(Controller $controller) $using_validation = false; if ($controller->policy() && !$controller->policy()->authorizeResource()) { - if (in_array($name, $controller->policy()->methods())) { - $body .= str_replace( + if (in_array(Policy::$resourceAbilityMap[$name], $controller->policy()->methods())) { + $body .= self::INDENT . str_replace( [ '{{ method }}', '{{ modelClass }}', @@ -107,8 +107,10 @@ protected function buildMethods(Controller $controller) Str::studly($controllerModelName), '$' . Str::camel($controllerModelName), ], - $this->filesystem->stub(Policy::$resourceAbilityStubMap[$name]) - ) . PHP_EOL; + in_array($name, ['index', 'create', 'store']) + ? "\$this->authorize('{{ method }}', {{ modelClass }}::class);" + : "\$this->authorize('{{ method }}', {{ modelVariable }});" + ) . PHP_EOL . PHP_EOL; } } diff --git a/src/Generators/PolicyGenerator.php b/src/Generators/PolicyGenerator.php index 8042c22b..90330308 100644 --- a/src/Generators/PolicyGenerator.php +++ b/src/Generators/PolicyGenerator.php @@ -56,10 +56,10 @@ protected function buildMethods(Policy $policy) Str::studly($policy->name()), Str::camel($policy->name()), ], - $this->filesystem->stub('policy.method.' . Policy::$resourceAbilityMap[$name] . '.stub'), + $this->filesystem->stub('policy.method.' . $name . '.stub'), ) . PHP_EOL; } - return $methods; + return trim($methods); } } diff --git a/src/Lexers/ControllerLexer.php b/src/Lexers/ControllerLexer.php index 4c97a90b..0081b213 100644 --- a/src/Lexers/ControllerLexer.php +++ b/src/Lexers/ControllerLexer.php @@ -62,8 +62,13 @@ public function analyze(array $tokens): array $policy = new Policy( $controller->prefix(), $authorizeResource === true - ? array_keys(Policy::$resourceAbilityMap) - : preg_split('/,([ \t]+)?/', $definition['meta']['policies']), + ? Policy::$supportedMethods + : array_unique( + array_map( + fn (string $method): string => Policy::$resourceAbilityMap[$method], + preg_split('/,([ \t]+)?/', $definition['meta']['policies']) + ) + ), $authorizeResource === true, ); diff --git a/src/Models/Policy.php b/src/Models/Policy.php index 2eadaa97..aeb81734 100644 --- a/src/Models/Policy.php +++ b/src/Models/Policy.php @@ -29,17 +29,6 @@ class Policy implements BlueprintModel 'destroy' => 'delete', ]; - /** @var array */ - public static $resourceAbilityStubMap = [ - 'index' => 'controller.policy-without-model.stub', - 'show' => 'controller.policy-without-model.stub', - 'create' => 'controller.policy-without-model.stub', - 'store' => 'controller.policy-without-model.stub', - 'edit' => 'controller.policy-with-model.stub', - 'update' => 'controller.policy-with-model.stub', - 'destroy' => 'controller.policy-with-model.stub', - ]; - /** * @var string */ diff --git a/stubs/controller.policy-with-model.stub b/stubs/controller.policy-with-model.stub deleted file mode 100644 index 9381e7e8..00000000 --- a/stubs/controller.policy-with-model.stub +++ /dev/null @@ -1 +0,0 @@ - $this->authorize('{{ method }}', {{ modelVariable }}); diff --git a/stubs/controller.policy-without-model.stub b/stubs/controller.policy-without-model.stub deleted file mode 100644 index 48f15788..00000000 --- a/stubs/controller.policy-without-model.stub +++ /dev/null @@ -1 +0,0 @@ - $this->authorize('{{ method }}', {{ modelClass }}::class); diff --git a/tests/Feature/Generators/ControllerGeneratorTest.php b/tests/Feature/Generators/ControllerGeneratorTest.php index 58c5f11e..0d21b571 100644 --- a/tests/Feature/Generators/ControllerGeneratorTest.php +++ b/tests/Feature/Generators/ControllerGeneratorTest.php @@ -166,14 +166,6 @@ public function output_generates_controller_with_all_policies(): void $this->filesystem->expects('stub') ->with('controller.method.stub') ->andReturn($this->stub('controller.method.stub')); - $this->filesystem->expects('stub') - ->times(3) - ->with('controller.policy-with-model.stub') - ->andReturn($this->stub('controller.policy-with-model.stub')); - $this->filesystem->expects('stub') - ->times(4) - ->with('controller.policy-without-model.stub') - ->andReturn($this->stub('controller.policy-without-model.stub')); $this->filesystem->expects('exists') ->with(dirname($path)) @@ -199,10 +191,6 @@ public function output_generates_controller_with_some_policies(): void $this->filesystem->expects('stub') ->with('controller.method.stub') ->andReturn($this->stub('controller.method.stub')); - $this->filesystem->expects('stub') - ->times(2) - ->with('controller.policy-without-model.stub') - ->andReturn($this->stub('controller.policy-without-model.stub')); $this->filesystem->expects('exists') ->with(dirname($path)) @@ -231,14 +219,6 @@ public function output_generates_controller_with_authorize_resource(): void $this->filesystem->expects('stub') ->with('controller.authorize-resource.stub') ->andReturn($this->stub('controller.authorize-resource.stub')); - $this->filesystem->expects('stub') - ->times(0) - ->with('controller.policy-with-model.stub') - ->andReturn($this->stub('controller.policy-with-model.stub')); - $this->filesystem->expects('stub') - ->times(0) - ->with('controller.policy-without-model.stub') - ->andReturn($this->stub('controller.policy-without-model.stub')); $this->filesystem->expects('exists') ->with(dirname($path)) diff --git a/tests/Feature/Generators/PolicyGeneratorTest.php b/tests/Feature/Generators/PolicyGeneratorTest.php index 36017158..b69c1ae0 100644 --- a/tests/Feature/Generators/PolicyGeneratorTest.php +++ b/tests/Feature/Generators/PolicyGeneratorTest.php @@ -62,12 +62,6 @@ public function output_generates_policies_for_controller_with_all_policies(): vo $this->filesystem->expects('stub') ->with('policy.method.create.stub') ->andReturn($this->stub('policy.method.create.stub')); - $this->filesystem->expects('stub') - ->with('policy.method.create.stub') - ->andReturn($this->stub('policy.method.create.stub')); - $this->filesystem->expects('stub') - ->with('policy.method.update.stub') - ->andReturn($this->stub('policy.method.update.stub')); $this->filesystem->expects('stub') ->with('policy.method.update.stub') ->andReturn($this->stub('policy.method.update.stub')); @@ -135,12 +129,6 @@ public function output_generates_policies_for_controller_with_authorize_resource $this->filesystem->expects('stub') ->with('policy.method.create.stub') ->andReturn($this->stub('policy.method.create.stub')); - $this->filesystem->expects('stub') - ->with('policy.method.create.stub') - ->andReturn($this->stub('policy.method.create.stub')); - $this->filesystem->expects('stub') - ->with('policy.method.update.stub') - ->andReturn($this->stub('policy.method.update.stub')); $this->filesystem->expects('stub') ->with('policy.method.update.stub') ->andReturn($this->stub('policy.method.update.stub')); diff --git a/tests/Feature/Lexers/ControllerLexerTest.php b/tests/Feature/Lexers/ControllerLexerTest.php index 6c40d3a3..a19bea86 100644 --- a/tests/Feature/Lexers/ControllerLexerTest.php +++ b/tests/Feature/Lexers/ControllerLexerTest.php @@ -459,7 +459,7 @@ public function it_returns_an_authorized_controller_with_all_policies(): void $controller = $actual['controllers']['Report']; $this->assertInstanceOf(Policy::class, $controller->policy()); - $this->assertEquals(array_keys(Policy::$resourceAbilityMap), $controller->policy()->methods()); + $this->assertEquals(Policy::$supportedMethods, $controller->policy()->methods()); } #[Test] @@ -484,6 +484,6 @@ public function it_returns_an_authorized_controller_with_specific_policies(): vo $controller = $actual['controllers']['Report']; $this->assertInstanceOf(Policy::class, $controller->policy()); - $this->assertEquals(['index', 'show'], $controller->policy()->methods()); + $this->assertEquals(['viewAny', 'view'], $controller->policy()->methods()); } } diff --git a/tests/fixtures/controllers/with-all-policies.php b/tests/fixtures/controllers/with-all-policies.php index 65901098..26453beb 100644 --- a/tests/fixtures/controllers/with-all-policies.php +++ b/tests/fixtures/controllers/with-all-policies.php @@ -41,7 +41,7 @@ public function store(PostStoreRequest $request): RedirectResponse public function show(Request $request, Post $post): View { - $this->authorize('show', Post::class); + $this->authorize('show', $post); return view('post.show', compact('post')); } diff --git a/tests/fixtures/controllers/with-some-policies.php b/tests/fixtures/controllers/with-some-policies.php index eb7b46b5..34e2a725 100644 --- a/tests/fixtures/controllers/with-some-policies.php +++ b/tests/fixtures/controllers/with-some-policies.php @@ -36,7 +36,7 @@ public function store(PostStoreRequest $request): RedirectResponse public function show(Request $request, Post $post): View { - $this->authorize('show', Post::class); + $this->authorize('show', $post); return view('post.show', compact('post')); } diff --git a/tests/fixtures/policies/with-all-policies.php b/tests/fixtures/policies/with-all-policies.php index 8aae21df..5beb9ae0 100644 --- a/tests/fixtures/policies/with-all-policies.php +++ b/tests/fixtures/policies/with-all-policies.php @@ -8,7 +8,7 @@ class PostPolicy { - /** + /** * Determine whether the user can view any models. */ public function viewAny(User $user): bool @@ -24,14 +24,6 @@ public function create(User $user): bool // } - /** - * Determine whether the user can create models. - */ - public function create(User $user): bool - { - // - } - /** * Determine whether the user can view the model. */ @@ -48,14 +40,6 @@ public function update(User $user, Post $post): bool // } - /** - * Determine whether the user can update the model. - */ - public function update(User $user, Post $post): bool - { - // - } - /** * Determine whether the user can delete the model. */ @@ -63,6 +47,4 @@ public function delete(User $user, Post $post): bool { // } - - } diff --git a/tests/fixtures/policies/with-authorize-resource.php b/tests/fixtures/policies/with-authorize-resource.php index 83f38ae7..4629a980 100644 --- a/tests/fixtures/policies/with-authorize-resource.php +++ b/tests/fixtures/policies/with-authorize-resource.php @@ -8,7 +8,7 @@ class PostPolicy { - /** + /** * Determine whether the user can view any models. */ public function viewAny(User $user): bool @@ -32,22 +32,6 @@ public function create(User $user): bool // } - /** - * Determine whether the user can create models. - */ - public function create(User $user): bool - { - // - } - - /** - * Determine whether the user can update the model. - */ - public function update(User $user, Post $post): bool - { - // - } - /** * Determine whether the user can update the model. */ @@ -63,6 +47,4 @@ public function delete(User $user, Post $post): bool { // } - - } diff --git a/tests/fixtures/policies/with-some-policies.php b/tests/fixtures/policies/with-some-policies.php index 3a7218f8..35fabd2f 100644 --- a/tests/fixtures/policies/with-some-policies.php +++ b/tests/fixtures/policies/with-some-policies.php @@ -8,7 +8,7 @@ class PostPolicy { - /** + /** * Determine whether the user can view any models. */ public function viewAny(User $user): bool @@ -23,6 +23,4 @@ public function view(User $user, Post $post): bool { // } - - }