From 1eb00efc6ea1fc4a093d905feaf95b4cb8fb1db4 Mon Sep 17 00:00:00 2001 From: Dalibor Korpar Date: Fri, 7 May 2021 10:56:02 +0200 Subject: [PATCH 1/4] use HttpMethod attribute instead of @httpMethod annotation --- composer.json | 2 +- src/Attributes/HttpMethod.php | 15 +++++++++ src/Controllers/Controller.php | 57 +++++++++++++++++++++------------- 3 files changed, 52 insertions(+), 22 deletions(-) create mode 100644 src/Attributes/HttpMethod.php diff --git a/composer.json b/composer.json index a4532fc..3e628cd 100644 --- a/composer.json +++ b/composer.json @@ -9,7 +9,7 @@ "nette/application": "^3.0", "nette/forms": "^3.1.2", "nette/reflection": "^2.4", - "wedo/utilities": "^1.0 || ^2.0" + "wedo/utilities": "^2.0" }, "require-dev": { "phpunit/phpunit": "^9.0", diff --git a/src/Attributes/HttpMethod.php b/src/Attributes/HttpMethod.php new file mode 100644 index 0000000..a49a08e --- /dev/null +++ b/src/Attributes/HttpMethod.php @@ -0,0 +1,15 @@ +value = $value; + } +} diff --git a/src/Controllers/Controller.php b/src/Controllers/Controller.php index 909204f..37ecbb5 100644 --- a/src/Controllers/Controller.php +++ b/src/Controllers/Controller.php @@ -8,7 +8,6 @@ use Nette\Application\Responses\JsonResponse; use Nette\Application\UI\Presenter; use Nette\Localization\Translator; -use Nette\Reflection\Annotation; use Nette\Utils\Json; use Nette\Utils\Strings; use Psr\Log\LoggerInterface; @@ -17,11 +16,11 @@ use ReflectionNamedType; use ReflectionParameter; use Throwable; +use Wedo\Api\Attributes\HttpMethod; use Wedo\Api\Exceptions\BadRequestException; use Wedo\Api\Exceptions\NotFoundException; use Wedo\Api\Exceptions\ResponseException; use Wedo\Api\Exceptions\ValidationException; -use Wedo\Api\Helpers\AnnotationHelper; use Wedo\Api\Requests\BaseRequest; use Wedo\Api\Responses\BaseResponse; use Wedo\Api\Responses\ErrorResponse; @@ -107,25 +106,7 @@ protected function tryCall(string $method, array $params): bool */ protected function validateRequest(ReflectionMethod $rm): void { - /** @var Annotation $expectedMethod */ - $expectedMethod = AnnotationHelper::getAnnotation($rm, 'httpMethod'); - $expectedMethod = strtoupper((string) $expectedMethod); - - if ($expectedMethod === '') { - $expectedMethod = 'get'; - $params = $rm->getParameters(); - /** @phpstan-ignore-next-line */ - if (!empty($params)) { - /** @var ReflectionNamedType|null $paramClass */ - $paramClass = $params[0]->getType(); - - if ($paramClass !== null) { - $paramClassName = $paramClass->getName(); - if (class_exists($paramClassName) && (new ReflectionClass($paramClassName))->isSubclassOf(BaseRequest::class)) - $expectedMethod = 'post'; - } - } - } + $expectedMethod = $this->getExpectedHttpMethod($rm); $request = $this->getHttpRequest(); @@ -256,4 +237,38 @@ public function injectTranslator(Translator $translator): void $this->translator = $translator; } + + private function getExpectedHttpMethod(ReflectionMethod $rm): string + { + $attributes = $rm->getAttributes('HttpMethod'); + + if (count($attributes) > 0) { + /** @var HttpMethod $httpMethod */ + $httpMethod = $attributes[0]->newInstance(); + return Strings::upper($httpMethod->value); + + + } + + $params = $rm->getParameters(); + + if (count($params) === 0) { + return 'GET'; + } + + /** @var ReflectionNamedType|null $paramClass */ + $paramClass = $params[0]->getType(); + + if ($paramClass === NULL) { + return 'GET'; + } + + $paramClassName = $paramClass->getName(); + if (class_exists($paramClassName) && (new ReflectionClass($paramClassName))->isSubclassOf(BaseRequest::class)) { + return 'POST'; + } + + return 'GET'; + } + } From f8a65434a355e305203739f1f7907be83ab2880a Mon Sep 17 00:00:00 2001 From: Dalibor Korpar Date: Fri, 7 May 2021 11:19:06 +0200 Subject: [PATCH 2/4] Switch internal annotation to php8 attribute/apply contributte coding styles --- .github/workflows/main.yml | 12 ++- composer.json | 4 +- ruleset.xml | 4 +- src/Attributes/HttpMethod.php | 3 + src/Attributes/Internal.php | 11 +++ src/Controllers/Controller.php | 43 ++++------- src/DI/ApiExtension.php | 4 +- src/Exceptions/BadRequestException.php | 3 +- src/Exceptions/ResponseException.php | 8 +- src/Exceptions/ValidationException.php | 3 +- src/Helpers/AnnotationHelper.php | 23 ------ src/Helpers/FormBuilder.php | 10 +-- src/Helpers/RequestHydrator.php | 12 +-- src/Requests/BaseRequest.php | 93 ++++++++++------------- src/Responses/ValidationErrorResponse.php | 1 + src/Routing/ApiRoute.php | 3 - tests/Requests/RequestTest.php | 3 - tests/Routing/RoutingTest.php | 4 +- 18 files changed, 92 insertions(+), 152 deletions(-) create mode 100644 src/Attributes/Internal.php delete mode 100644 src/Helpers/AnnotationHelper.php diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 26a634c..6194505 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -23,7 +23,7 @@ jobs: strategy: matrix: - php-version: ["7.4"] + php-version: ["8.0"] operating-system: ["ubuntu-latest"] fail-fast: false @@ -80,11 +80,11 @@ jobs: runs-on: "${{ matrix.operating-system }}" strategy: matrix: - php-version: ["7.4"] + php-version: ["8.0"] operating-system: ["ubuntu-latest"] composer-args: [ "" ] include: - - php-version: "7.4" + - php-version: "8.0" operating-system: "ubuntu-latest" composer-args: "--prefer-lowest" - php-version: "8.0" @@ -92,8 +92,6 @@ jobs: composer-args: "--ignore-platform-reqs" fail-fast: false - continue-on-error: "${{ matrix.php-version == '8.0' }}" - steps: - name: "Checkout" uses: "actions/checkout@v2" @@ -149,7 +147,7 @@ jobs: strategy: matrix: - php-version: ["7.4"] + php-version: ["8.0"] operating-system: ["ubuntu-latest"] fail-fast: false @@ -205,7 +203,7 @@ jobs: strategy: matrix: - php-version: ["7.4"] + php-version: ["8.0"] operating-system: ["ubuntu-latest"] fail-fast: false diff --git a/composer.json b/composer.json index 3e628cd..e525539 100644 --- a/composer.json +++ b/composer.json @@ -3,7 +3,7 @@ "description": "rest api", "license": ["MIT"], "require": { - "php": "^7.4 || ^8.0", + "php": "^8.0", "psr/log": "^1.1", "nette/di": "^3.0", "nette/application": "^3.0", @@ -18,7 +18,7 @@ "phpstan/phpstan-dibi": "^0.12", "phpstan/phpstan-nette": "^0.12", "phpstan/phpstan-strict-rules": "^0.12", - "ninjify/qa": "^0.12" + "contributte/qa": "^0.1" }, "autoload": { "psr-4": { diff --git a/ruleset.xml b/ruleset.xml index 0c46aba..d8516d1 100644 --- a/ruleset.xml +++ b/ruleset.xml @@ -1,8 +1,8 @@ - - + + diff --git a/src/Attributes/HttpMethod.php b/src/Attributes/HttpMethod.php index a49a08e..11c1252 100644 --- a/src/Attributes/HttpMethod.php +++ b/src/Attributes/HttpMethod.php @@ -2,6 +2,8 @@ namespace Wedo\Api\Attributes; +use Attribute; + #[Attribute] class HttpMethod { @@ -12,4 +14,5 @@ public function __construct(string $value) { $this->value = $value; } + } diff --git a/src/Attributes/Internal.php b/src/Attributes/Internal.php new file mode 100644 index 0000000..18802cc --- /dev/null +++ b/src/Attributes/Internal.php @@ -0,0 +1,11 @@ +payload; + $result ??= $this->payload; $this->setTranslatorOnJsonTranslatable($result); return new JsonResponse($result ?? $this->payload); } + public function injectTranslator(Translator $translator): void + { + $translator = clone $translator; + $this->translator = $translator; + } /** * @param mixed[] $params @@ -89,18 +93,13 @@ protected function tryCall(string $method, array $params): bool $this->validateRequest($rm); $methodParams = $rm->getParameters(); - if (count($methodParams) > 0) { - $params = $this->createParams($params, $methodParams); - } else { - $params = []; - } + $params = count($methodParams) > 0 ? $this->createParams($params, $methodParams) : []; $this->payload = $rm->invokeArgs($this, $params); return true; } - /** * @throws BadRequestException */ @@ -117,7 +116,6 @@ protected function validateRequest(ReflectionMethod $rm): void } } - /** * @param mixed[] $params * @param ReflectionParameter[] $methodParams @@ -158,7 +156,6 @@ protected function createParams(array $params, array $methodParams): array return $params; } - protected function process(): ?BaseResponse { try { @@ -184,12 +181,11 @@ protected function process(): ?BaseResponse return $result; } - protected function beforeProcess(): void { + //override if needed } - protected function setOptionsRequestHeaders(): void { $this->getHttpResponse()->addHeader('Access-Control-Allow-Headers', 'api-key, accept, Content-Type, session-id'); @@ -198,11 +194,10 @@ protected function setOptionsRequestHeaders(): void $this->getHttpResponse()->setCode(200); } - /** * @param BaseResponse|JsonObject|mixed[] $data */ - private function setTranslatorOnJsonTranslatable(&$data): void + private function setTranslatorOnJsonTranslatable(mixed $data): void { if (is_array($data)) { foreach ($data as $key => $value) { @@ -231,23 +226,15 @@ private function setTranslatorOnJsonTranslatable(&$data): void } } - public function injectTranslator(Translator $translator): void - { - $translator = clone $translator; - $this->translator = $translator; - } - - private function getExpectedHttpMethod(ReflectionMethod $rm): string { - $attributes = $rm->getAttributes('HttpMethod'); + $attributes = $rm->getAttributes(HttpMethod::class); if (count($attributes) > 0) { /** @var HttpMethod $httpMethod */ $httpMethod = $attributes[0]->newInstance(); - return Strings::upper($httpMethod->value); - + return Strings::upper($httpMethod->value); } $params = $rm->getParameters(); @@ -259,7 +246,7 @@ private function getExpectedHttpMethod(ReflectionMethod $rm): string /** @var ReflectionNamedType|null $paramClass */ $paramClass = $params[0]->getType(); - if ($paramClass === NULL) { + if ($paramClass === null) { return 'GET'; } diff --git a/src/DI/ApiExtension.php b/src/DI/ApiExtension.php index ac95eb8..b0ebea4 100644 --- a/src/DI/ApiExtension.php +++ b/src/DI/ApiExtension.php @@ -5,10 +5,10 @@ use Nette\Application\Routers\Route; use Nette\DI\CompilerExtension; use Nette\DI\Definitions\ServiceDefinition; -use Nette\Reflection\AnnotationsParser; use Nette\Utils\Strings; use ReflectionClass; use ReflectionMethod; +use Wedo\Api\Attributes\Internal; use Wedo\Api\Routing\RouterFactory; /** @@ -58,7 +58,7 @@ public function beforeCompile(): void foreach ($publicMethods as $oneMethod) { if ($oneMethod->isConstructor() || - isset(AnnotationsParser::getAll($oneMethod)['internal']) || + count($oneMethod->getAttributes(Internal::class)) > 0 || $oneMethod->getDeclaringClass()->getName() !== $obj->getName()) { continue; } diff --git a/src/Exceptions/BadRequestException.php b/src/Exceptions/BadRequestException.php index 5266eff..ccd1281 100644 --- a/src/Exceptions/BadRequestException.php +++ b/src/Exceptions/BadRequestException.php @@ -8,10 +8,9 @@ class BadRequestException extends ResponseException { /** - * @param mixed|string ...$parameters * @codeCoverageIgnore */ - public function __construct(string $message = '', int $code = 400, ?Throwable $previous = null, ...$parameters) + public function __construct(string $message = '', int $code = 400, ?Throwable $previous = null, mixed ...$parameters) { parent::__construct($message, $code, $previous, $parameters); } diff --git a/src/Exceptions/ResponseException.php b/src/Exceptions/ResponseException.php index f36bff0..2866d5d 100644 --- a/src/Exceptions/ResponseException.php +++ b/src/Exceptions/ResponseException.php @@ -15,16 +15,13 @@ class ResponseException extends Exception /** @var ResponseException[] */ protected array $additionalExceptions = []; - /** - * @param mixed|string ...$parameters - */ - public function __construct(string $message = '', int $code = 500, ?Throwable $previous = null, ...$parameters) + public function __construct(string $message = '', int $code = 500, ?Throwable $previous = null, mixed ...$parameters) { $this->parameters = $parameters; + parent::__construct($message, $code, $previous); } - public function getTranslatedMessage(ITranslator $translator): string { return $translator->translate($this->getMessage(), $this->parameters); @@ -52,7 +49,6 @@ public function getAll(): array return [...[$this], ...$this->getAdditionalExceptions()]; } - /** * @return string[] */ diff --git a/src/Exceptions/ValidationException.php b/src/Exceptions/ValidationException.php index 207698f..02656fd 100644 --- a/src/Exceptions/ValidationException.php +++ b/src/Exceptions/ValidationException.php @@ -16,10 +16,10 @@ class ValidationException extends ResponseException public function __construct(array $validationErrors) { parent::__construct('Data is not Valid!', 422); + $this->validationErrors = $validationErrors; } - /** * @return ValidationError[] * @codeCoverageIgnore @@ -29,7 +29,6 @@ public function getValidationErrors(): array return $this->validationErrors; } - /** * @param ValidationError[] $validationErrors * @codeCoverageIgnore diff --git a/src/Helpers/AnnotationHelper.php b/src/Helpers/AnnotationHelper.php deleted file mode 100644 index ee0e47d..0000000 --- a/src/Helpers/AnnotationHelper.php +++ /dev/null @@ -1,23 +0,0 @@ -isBuiltin() && $type->getName() !== 'DateTimeInterface') { throw new InvalidArgumentException('Only built in types are supported! ' . $type->getName() . ' is not supported'); @@ -22,11 +18,7 @@ public static function hydrateValue(ReflectionNamedType $type, $value) return self::castValueToBuiltInType($value, $type->getName(), $type->allowsNull()); } - /** - * @param mixed $value - * @return int|float|string|bool|DateTimeImmutable|null - */ - public static function castValueToBuiltInType($value, string $type, bool $allowsNull) + public static function castValueToBuiltInType(mixed $value, string $type, bool $allowsNull): int|float|string|bool|DateTimeImmutable|null { switch ($type) { case 'int': diff --git a/src/Requests/BaseRequest.php b/src/Requests/BaseRequest.php index dc415c9..e7d4a4b 100644 --- a/src/Requests/BaseRequest.php +++ b/src/Requests/BaseRequest.php @@ -42,7 +42,6 @@ public function __construct(?FormBuilder $formBuilder = null) $this->formBuilder = $formBuilder ?? new FormBuilder(); } - /** * @param mixed[] $data * @param Container $form ($this->form by default) @@ -60,7 +59,6 @@ public function buildForm(array $data = [], ?Container $form = null, ?BaseReques $request->setValues($data); } - /** * Read annotations from public properties in current class * @@ -79,7 +77,6 @@ public function getAnnotations(): ?array return $annotations ?? null; } - /** * @internal * @param string[][] $annotations @@ -112,7 +109,7 @@ public function setValidationRule(string $annotation, BaseControl $control, arra case 'items': if (!$control instanceof ChoiceControl) { throw new NotSupportedException('Items annotation cannot be set on on control type "' - . get_class($control) . '" in ' . static::class); + . $control::class . '" in ' . static::class); } $this->setItems($control, $args); @@ -138,7 +135,6 @@ public function setValues(array $values): void $this->fillFromForm(); } - /** * @return array */ @@ -166,7 +162,6 @@ public function toArray(): array return $arr; } - /** * @param mixed[] $values * @return string[][] @@ -186,7 +181,6 @@ public function valuesToString(array $values): array return $values; } - /** * @internal */ @@ -209,7 +203,6 @@ public function fillFromForm(): void } } - /** * @internal */ @@ -218,7 +211,6 @@ public function isValid(): bool return $this->form->isValid(); } - /** * @throws ValidationException */ @@ -237,7 +229,6 @@ public function validate(): void throw new ValidationException($errors); } - /** * Returns array in control => error way * @@ -280,6 +271,44 @@ public function getErrors(): array return $errors; } + public function getForm(): Container + { + return $this->form; + } + + /** + * @return ReflectionProperty[] + */ + public function getReflectionProperties(): array + { + if (!isset($this->reflectionProperties)) { + $properties = (new ReflectionObject($this))->getProperties(ReflectionProperty::IS_PUBLIC); + $this->reflectionProperties = []; + + foreach ($properties as $property) { + $this->reflectionProperties[$property->getName()] = $property; + } + } + + return $this->reflectionProperties; + } + + protected function setForm(?Container $form = null): Container + { + if ($form === null) { + $form = $this->formBuilder->createEmptyForm(); + } + + $this->form = $form; + + if ($form instanceof Form) { + $form->onSubmit[] = function (): void { + $this->fillFromForm(); + }; + } + + return $form; + } /** * @param mixed[] $args @@ -298,7 +327,6 @@ private function addRule(BaseControl $control, array $args): void } } - /** * @param mixed[] $args */ @@ -310,7 +338,6 @@ private function addCustomRule(BaseControl $control, array $args): void } } - /** * @param mixed[] $args */ @@ -319,30 +346,6 @@ private function addRequiredRule(BaseControl $control, array $args): void call_user_func_array([$control->getRules(), 'setRequired'], $args); } - - public function getForm(): Container - { - return $this->form; - } - - protected function setForm(?Container $form = null): Container - { - if ($form === null) { - $form = $this->formBuilder->createEmptyForm(); - } - - $this->form = $form; - - if ($form instanceof Form) { - $form->onSubmit[] = function (): void { - $this->fillFromForm(); - }; - } - - return $form; - } - - /** * @param mixed[] $args * @throws NotSupportedException @@ -357,22 +360,4 @@ private function setItems(ChoiceControl $control, array $args): void } } - - /** - * @return ReflectionProperty[] - */ - public function getReflectionProperties(): array - { - if (!isset($this->reflectionProperties)) { - $properties = (new ReflectionObject($this))->getProperties(ReflectionProperty::IS_PUBLIC); - $this->reflectionProperties = []; - - foreach ($properties as $property) { - $this->reflectionProperties[$property->getName()] = $property; - } - } - - return $this->reflectionProperties; - } - } diff --git a/src/Responses/ValidationErrorResponse.php b/src/Responses/ValidationErrorResponse.php index 92729d5..3a753ae 100644 --- a/src/Responses/ValidationErrorResponse.php +++ b/src/Responses/ValidationErrorResponse.php @@ -19,6 +19,7 @@ class ValidationErrorResponse extends ErrorResponse public function __construct(ValidationException $exception) { parent::__construct($exception); + $this->validation_errors = $exception->getValidationErrors(); } diff --git a/src/Routing/ApiRoute.php b/src/Routing/ApiRoute.php index b457f9d..0a6d9bb 100644 --- a/src/Routing/ApiRoute.php +++ b/src/Routing/ApiRoute.php @@ -28,7 +28,6 @@ public function __construct(string $prefix, string $controllerNamespace, array $ $this->apiEndPoints = $apiEndpoints; } - /** * @return mixed[]|null */ @@ -62,7 +61,6 @@ public function match(IRequest $context): ?array return $params; } - /** * Constructs absolute URL from Request object. Not implemented for API, since its not needed * @@ -73,7 +71,6 @@ public function constructUrl(array $appRequest, Nette\Http\UrlScript $refUrl): ? return null; } - /** * @param mixed[] $paramsPart * @return array [$presenter, $action, mixed] diff --git a/tests/Requests/RequestTest.php b/tests/Requests/RequestTest.php index e6a43c5..c8a2f4b 100644 --- a/tests/Requests/RequestTest.php +++ b/tests/Requests/RequestTest.php @@ -52,7 +52,6 @@ public function testNoVarAnnotationSet(): void $request->buildForm(); } - public function testSetRequiredFalse(): void { $request = new RequiredFalseRequest(); @@ -89,7 +88,6 @@ public function testNotBuiltInType(): void $request->buildForm(); } - public function testArrayRequest_WithValidData_ReturnsTrue(): void { $request = new TestArrayRequest(); @@ -130,7 +128,6 @@ public function testArrayRequest_WithInvalidData_ReturnsFalse(): void $this->assertEquals($data, $request->toArray()); } - public function testContainer_WithValidJson(): void { $json = '{"items":[{"name":"bla"}]}'; diff --git a/tests/Routing/RoutingTest.php b/tests/Routing/RoutingTest.php index 5ca0ffd..8349c59 100644 --- a/tests/Routing/RoutingTest.php +++ b/tests/Routing/RoutingTest.php @@ -42,24 +42,22 @@ public function testMatch_WithNotExistingService_ShoulgReturnNull(): void $this->assertNull($route->match($this->createRequest('/api/v1/route1/not-exist'))); } - public function testMatchNotApiUrl(): void { $route = new ApiRoute('/api/v1', 'Api', []); $this->assertNull($route->match($this->createRequest('/api/v'))); } - public function testConstruct(): void { $route = new ApiRoute('/api/v1', 'Api', []); $this->assertNull($route->constructUrl(['presenter' => 'Api:Default'], new UrlScript('http://domain.local'))); } - private function createRequest(string $path): Request { $url = new UrlScript('http://domain.local' . $path); + return new Request($url); } From 28941c1080e5596df7bf3432cef2d7b7383de884 Mon Sep 17 00:00:00 2001 From: Dalibor Korpar Date: Wed, 12 May 2021 09:53:30 +0200 Subject: [PATCH 3/4] Switch annotations for controls/validations to php8 attribute --- src/Attributes/ChoiceControlItems.php | 25 ++++ src/Attributes/ContainerType.php | 22 ++++ src/Attributes/Control.php | 34 +++++ src/Attributes/RequiredRule.php | 18 +++ src/Attributes/ValidationRule.php | 25 ++++ src/Helpers/FormBuilder.php | 79 +++++++---- src/Requests/BaseRequest.php | 124 ++---------------- tests/Requests/RequestTest.php | 18 +-- tests/Requests/RequiredFalseRequest.php | 8 +- tests/Requests/SimpleRequest.php | 8 +- tests/Requests/SimpleRequest2.php | 3 +- tests/Requests/TestArrayRequest.php | 20 +-- tests/Requests/TestItemsNotJsonRequest.php | 16 --- tests/Requests/TestItemsRequest.php | 11 +- tests/Requests/TestNonExistingRuleRequest.php | 8 +- tests/Requests/TestNotBuiltInTypeRequest.php | 3 +- tests/Requests/TestRequest.php | 34 +++-- tests/Requests/TestWithItemsOnTextRequest.php | 16 --- 18 files changed, 239 insertions(+), 233 deletions(-) create mode 100644 src/Attributes/ChoiceControlItems.php create mode 100644 src/Attributes/ContainerType.php create mode 100644 src/Attributes/Control.php create mode 100644 src/Attributes/RequiredRule.php create mode 100644 src/Attributes/ValidationRule.php delete mode 100644 tests/Requests/TestItemsNotJsonRequest.php delete mode 100644 tests/Requests/TestWithItemsOnTextRequest.php diff --git a/src/Attributes/ChoiceControlItems.php b/src/Attributes/ChoiceControlItems.php new file mode 100644 index 0000000..472220f --- /dev/null +++ b/src/Attributes/ChoiceControlItems.php @@ -0,0 +1,25 @@ +items = $items; + $this->useKeys = $useKeys; + } + +} diff --git a/src/Attributes/ContainerType.php b/src/Attributes/ContainerType.php new file mode 100644 index 0000000..a02d7a6 --- /dev/null +++ b/src/Attributes/ContainerType.php @@ -0,0 +1,22 @@ +value = $value; + } + +} diff --git a/src/Attributes/Control.php b/src/Attributes/Control.php new file mode 100644 index 0000000..64ac1a7 --- /dev/null +++ b/src/Attributes/Control.php @@ -0,0 +1,34 @@ +value = $value; + } + +} diff --git a/src/Attributes/RequiredRule.php b/src/Attributes/RequiredRule.php new file mode 100644 index 0000000..8dcb91a --- /dev/null +++ b/src/Attributes/RequiredRule.php @@ -0,0 +1,18 @@ +required = $required; + } + +} diff --git a/src/Attributes/ValidationRule.php b/src/Attributes/ValidationRule.php new file mode 100644 index 0000000..af628ae --- /dev/null +++ b/src/Attributes/ValidationRule.php @@ -0,0 +1,25 @@ +validator = $validator; + $this->errorMessage = $errorMessage; + $this->args = $args; + } + +} diff --git a/src/Helpers/FormBuilder.php b/src/Helpers/FormBuilder.php index 09e8ce1..bd81e2d 100644 --- a/src/Helpers/FormBuilder.php +++ b/src/Helpers/FormBuilder.php @@ -6,61 +6,81 @@ use Nette\ArgumentOutOfRangeException; use Nette\Forms\Container; use Nette\Forms\Controls\BaseControl; +use Nette\Forms\Controls\ChoiceControl; use Nette\NotSupportedException; +use ReflectionProperty; +use Throwable; +use Wedo\Api\Attributes\ChoiceControlItems; +use Wedo\Api\Attributes\ContainerType; +use Wedo\Api\Attributes\Control; use Wedo\Api\Requests\BaseRequest; -use Wedo\Utilities\ClassNameHelper; class FormBuilder { /** - * @param array $properties + * @param ReflectionProperty[] $properties * @param mixed[] $data */ public function createForm(array $properties, BaseRequest $request, Container $form, array $data): void { - /** - * @var string $property - * @var string[][] $annotations - */ - foreach ($properties as $property => $annotations) { - $controlType = $this->getControlType($annotations); + foreach ($properties as $property) { + $controlType = $this->getControlType($property); $controlAddCallback = [$form, 'add' . $controlType]; - if ((!method_exists($form, $controlAddCallback[1]) && $controlType !== 'Date') || !is_callable($controlAddCallback)) { - throw new NotSupportedException('Control of type ' . $controlType . ' does not exist!'); + try { + $control = $controlAddCallback($property->getName()); //@phpstan-ignore-line + } catch (Throwable $ex) { + throw new NotSupportedException('Cannot add control of type ' . $controlType, 0, $ex); } - $control = call_user_func($controlAddCallback, $property); - - if ($controlType === 'Container') { + if ($controlType === Control::CONTAINER) { /** @phpstan-ignore-next-line */ $request->$property = []; /** @phpstan-ignore-next-line */ - if (empty($data[$property])) { + if (empty($data[$property->getName()])) { continue; } - $values = $data[$property]; + $values = $data[$property->getName()]; + + $requestTypeAttributes = $property->getAttributes(ContainerType::class); + + if (count($requestTypeAttributes) === 0) { + throw new NotSupportedException('ContainerType attribute not found for ' . + $property->getDeclaringClass()->getName() . '::' . $property->getName()); + } + + /** @var ContainerType $requestTypeAttribute */ + $requestTypeAttribute = $requestTypeAttributes[0]->newInstance(); - $reqType = rtrim($annotations['var'][0], ']['); - $reqType = ClassNameHelper::extractFqnFromObjectUseStatements($request, $reqType); + $requestType = $requestTypeAttribute->value; foreach ($values as $key => $value) { /** @var Container $container */ $container = $control->addContainer($key); - $item = new $reqType(); + $item = new $requestType(); $item->buildForm($value, $container, $item); /** @phpstan-ignore-next-line */ - $request->$property[] = $item; + $request->{$property->getName()}[] = $item; } } - unset($annotations['var'], $annotations['description'], $annotations['control']); - if ($control instanceof BaseControl) { - $request->setValidationRules($annotations, $control); + $request->setValidationRules($property, $control); + } + + if ($control instanceof ChoiceControl) { + $itemsAttributes = $property->getAttributes(ChoiceControlItems::class); + + if (count($itemsAttributes) === 0) { + throw new NotSupportedException('Choice control must have ChoiceControlItems attribute set!'); + } + + /** @var ChoiceControlItems $itemAttribute */ + $itemAttribute = $itemsAttributes[0]->newInstance(); + $control->setItems($itemAttribute->items, $itemAttribute->useKeys); } } } @@ -71,17 +91,20 @@ public function createEmptyForm(): Form } /** - * @param string[][] $annotations * @throws ArgumentOutOfRangeException - * @throws NotSupportedException */ - protected function getControlType(array $annotations): string + protected function getControlType(ReflectionProperty $property): string { - if (!isset($annotations['control'])) { - throw new ArgumentOutOfRangeException('@control annotation not set on request!'); + $controlAttributes = $property->getAttributes(Control::class); + if (count($controlAttributes) === 0) { + throw new ArgumentOutOfRangeException('#[Control] Attribute not set on ' . + $property->getDeclaringClass()->getName() . '::' . $property->getName()); } - return $annotations['control'][0]; + /** @var Control $controlAttribute */ + $controlAttribute = $controlAttributes[0]->newInstance(); + + return $controlAttribute->value; } } diff --git a/src/Requests/BaseRequest.php b/src/Requests/BaseRequest.php index e7d4a4b..218a455 100644 --- a/src/Requests/BaseRequest.php +++ b/src/Requests/BaseRequest.php @@ -5,15 +5,14 @@ use Nette\ArgumentOutOfRangeException; use Nette\Forms\Container; use Nette\Forms\Controls\BaseControl; -use Nette\Forms\Controls\ChoiceControl; use Nette\Forms\Form; use Nette\NotSupportedException; -use Nette\Reflection\AnnotationsParser; use Nette\Utils\ArrayHash; -use Nette\Utils\Strings; use ReflectionNamedType; use ReflectionObject; use ReflectionProperty; +use Wedo\Api\Attributes\RequiredRule; +use Wedo\Api\Attributes\ValidationRule; use Wedo\Api\Entities\ValidationError; use Wedo\Api\Exceptions\ValidationException; use Wedo\Api\Helpers\FormBuilder; @@ -53,76 +52,31 @@ public function buildForm(array $data = [], ?Container $form = null, ?BaseReques $request ??= $this; $form = $request->setForm($form); - $properties = $request->getAnnotations() ?? []; + $properties = $request->getReflectionProperties(); $this->formBuilder->createForm($properties, $request, $form, $data); $request->setValues($data); } /** - * Read annotations from public properties in current class - * - * @return array|null * @internal */ - public function getAnnotations(): ?array + public function setValidationRules(ReflectionProperty $property, BaseControl $control): void { - $reflectionProperties = $this->getReflectionProperties(); - $annotations = []; + $validationRulesAttributes = $property->getAttributes(ValidationRule::class); - foreach ($reflectionProperties as $rp) { - $annotations[$rp->name] = AnnotationsParser::getAll($rp); + foreach ($validationRulesAttributes as $validationRuleAttribute) { + /** @var ValidationRule $validationRule */ + $validationRule = $validationRuleAttribute->newInstance(); + $control->addRule($validationRule->validator, $validationRule->errorMessage, $validationRule->args); } - return $annotations ?? null; - } - - /** - * @internal - * @param string[][] $annotations - */ - public function setValidationRules(array $annotations, BaseControl $control): void - { - foreach ($annotations as $annotation => $args) { - $this->setValidationRule($annotation, $control, $args); - } - } - - /** - * @param string[] $args - */ - public function setValidationRule(string $annotation, BaseControl $control, array $args): void - { - switch ($annotation) { - case 'addCustomRule': - $this->addCustomRule($control, $args); - - return; - case 'addRule': - $this->addRule($control, $args); - - return; - case 'setRequired': - $this->addRequiredRule($control, $args); - - return; - case 'items': - if (!$control instanceof ChoiceControl) { - throw new NotSupportedException('Items annotation cannot be set on on control type "' - . $control::class . '" in ' . static::class); - } - - $this->setItems($control, $args); - - return; - default: - $callable = [$control->getRules(), $annotation]; + $requiredRuleAttributes = $property->getAttributes(RequiredRule::class); - if (!method_exists($control->getRules(), $annotation) || !is_callable($callable)) { - throw new NotSupportedException('Cannot apply rule "' . $annotation . '"!'); - } - - call_user_func_array($callable, $args); + if (count($requiredRuleAttributes) > 0) { + /** @var RequiredRule $requiredRule */ + $requiredRule = $requiredRuleAttributes[0]->newInstance(); + $control->setRequired($requiredRule->required); } } @@ -310,54 +264,4 @@ protected function setForm(?Container $form = null): Container return $form; } - /** - * @param mixed[] $args - */ - private function addRule(BaseControl $control, array $args): void - { - foreach ($args as $arg) { - if ($arg instanceof ArrayHash) { - $addRuleArgs = (array) $arg; - array_unshift($addRuleArgs, constant(Form::class . '::' . strtoupper(array_shift($addRuleArgs)))); - } else { - $addRuleArgs = [constant(Form::class . '::' . strtoupper($arg))]; - } - - call_user_func_array([$control, 'addRule'], $addRuleArgs); - } - } - - /** - * @param mixed[] $args - */ - private function addCustomRule(BaseControl $control, array $args): void - { - foreach ($args as $arg) { - $addRuleArgs = $arg instanceof ArrayHash ? (array) $arg : $arg; - call_user_func_array([$control, 'addRule'], $addRuleArgs); - } - } - - /** - * @param mixed[] $args - */ - private function addRequiredRule(BaseControl $control, array $args): void - { - call_user_func_array([$control->getRules(), 'setRequired'], $args); - } - - /** - * @param mixed[] $args - * @throws NotSupportedException - */ - private function setItems(ChoiceControl $control, array $args): void - { - if (Strings::startsWith($args[0], '[') || Strings::startsWith($args[0], '{')) { - $items = json_decode($args[0], true); - $control->setItems($items); - } else { - throw new NotSupportedException('Only Json is allowed for setting items on select in request!'); - } - } - } diff --git a/tests/Requests/RequestTest.php b/tests/Requests/RequestTest.php index c8a2f4b..26e1499 100644 --- a/tests/Requests/RequestTest.php +++ b/tests/Requests/RequestTest.php @@ -62,21 +62,14 @@ public function testSetRequiredFalse(): void public function testNonExistingRule(): void { - $this->expectExceptionMessage('Cannot apply rule "addBlaSomething"!'); + $this->expectExceptionMessage('Unknown validator \'BlaSomething\' for control \'name\'.'); $request = new TestNonExistingRuleRequest(); $request->buildForm(); } - public function testItemsOnTextControlType_ShouldThrowException(): void - { - $this->expectExceptionMessage('Items annotation cannot be set on on control type'); - $request = new TestWithItemsOnTextRequest(); - $request->buildForm(); - } - public function testUnsupportedControlType(): void { - $this->expectExceptionMessage('Control of type nonExistingType123 does not exist!'); + $this->expectExceptionMessage('#[Control] Attribute not set on Wedo\Api\Tests\Requests\TestUnsupportedControlTypeRequest::name'); $request = new TestUnsupportedControlTypeRequest(); $request->buildForm(); } @@ -180,13 +173,6 @@ public function testValidate_WithValidData_ShouldReturnTrue(): void $this->assertEquals($request->name, 'Dalibor Korpar'); } - public function testItemsNotJsonRequest(): void - { - $this->expectExceptionMessage('Only Json is allowed for setting items on select in request!'); - $req = new TestItemsNotJsonRequest(); - $req->buildForm(); - } - public function testArrayRequest(): void { $json = '{"items":[{"name":"first", "name": null, "name": "bla"}]}'; diff --git a/tests/Requests/RequiredFalseRequest.php b/tests/Requests/RequiredFalseRequest.php index 2b2ccd1..2f0863a 100644 --- a/tests/Requests/RequiredFalseRequest.php +++ b/tests/Requests/RequiredFalseRequest.php @@ -2,15 +2,15 @@ namespace Wedo\Api\Tests\Requests; +use Wedo\Api\Attributes\Control; +use Wedo\Api\Attributes\RequiredRule; use Wedo\Api\Requests\BaseRequest; class RequiredFalseRequest extends BaseRequest { - /** - * @setRequired(FALSE) - * @control Text - */ + #[Control(Control::TEXT)] + #[RequiredRule(false)] public string $name; } diff --git a/tests/Requests/SimpleRequest.php b/tests/Requests/SimpleRequest.php index 801553a..1382ba9 100644 --- a/tests/Requests/SimpleRequest.php +++ b/tests/Requests/SimpleRequest.php @@ -2,15 +2,15 @@ namespace Wedo\Api\Tests\Requests; +use Wedo\Api\Attributes\Control; +use Wedo\Api\Attributes\RequiredRule; use Wedo\Api\Requests\BaseRequest; class SimpleRequest extends BaseRequest { - /** - * @setRequired - * @control Text - */ + #[Control(Control::TEXT)] + #[RequiredRule()] public string $name; } diff --git a/tests/Requests/SimpleRequest2.php b/tests/Requests/SimpleRequest2.php index 3521bd3..f23f27d 100644 --- a/tests/Requests/SimpleRequest2.php +++ b/tests/Requests/SimpleRequest2.php @@ -2,12 +2,13 @@ namespace Wedo\Api\Tests\Requests; +use Wedo\Api\Attributes\Control; use Wedo\Api\Requests\BaseRequest; class SimpleRequest2 extends BaseRequest { - /** @control Text */ + #[Control(Control::TEXT)] public string $name; } diff --git a/tests/Requests/TestArrayRequest.php b/tests/Requests/TestArrayRequest.php index d059c9a..3def550 100644 --- a/tests/Requests/TestArrayRequest.php +++ b/tests/Requests/TestArrayRequest.php @@ -2,22 +2,24 @@ namespace Wedo\Api\Tests\Requests; +use Nette\Forms\Form; +use Wedo\Api\Attributes\ContainerType; +use Wedo\Api\Attributes\Control; +use Wedo\Api\Attributes\ValidationRule; use Wedo\Api\Requests\BaseRequest; class TestArrayRequest extends BaseRequest { - /** - * @var SimpleRequest[] - * @control Container - * @setRequired() - */ + /** @var SimpleRequest[] $items */ + #[Control(Control::CONTAINER)] + #[ValidationRule(Form::REQUIRED)] + #[ContainerType(SimpleRequest::class)] public array $items; - /** - * @var SimpleRequest2[] - * @control Container - */ + /** @var SimpleRequest2[] $item2 */ + #[Control(Control::CONTAINER)] + #[ContainerType(SimpleRequest2::class)] public array $item2; } diff --git a/tests/Requests/TestItemsNotJsonRequest.php b/tests/Requests/TestItemsNotJsonRequest.php deleted file mode 100644 index 077aae8..0000000 --- a/tests/Requests/TestItemsNotJsonRequest.php +++ /dev/null @@ -1,16 +0,0 @@ - 1, 2, 3, 4, 5])] + #[RequiredRule()] public int $rating; } diff --git a/tests/Requests/TestNonExistingRuleRequest.php b/tests/Requests/TestNonExistingRuleRequest.php index c18eedf..91b0bfa 100644 --- a/tests/Requests/TestNonExistingRuleRequest.php +++ b/tests/Requests/TestNonExistingRuleRequest.php @@ -2,15 +2,15 @@ namespace Wedo\Api\Tests\Requests; +use Wedo\Api\Attributes\Control; +use Wedo\Api\Attributes\ValidationRule; use Wedo\Api\Requests\BaseRequest; class TestNonExistingRuleRequest extends BaseRequest { - /** - * @control Text - * @addBlaSomething() - */ + #[Control(Control::TEXT)] + #[ValidationRule('BlaSomething')] public string $name; } diff --git a/tests/Requests/TestNotBuiltInTypeRequest.php b/tests/Requests/TestNotBuiltInTypeRequest.php index b580ae9..bf615d2 100644 --- a/tests/Requests/TestNotBuiltInTypeRequest.php +++ b/tests/Requests/TestNotBuiltInTypeRequest.php @@ -2,6 +2,7 @@ namespace Wedo\Api\Tests\Requests; +use Wedo\Api\Attributes\Control; use Wedo\Api\Requests\BaseRequest; //phpcs:disable @@ -10,8 +11,8 @@ class TestNotBuiltInTypeRequest extends BaseRequest /** * typehint not set here and this should throw an exception - * @control Text */ + #[Control(Control::TEXT)] public \stdClass $name; } diff --git a/tests/Requests/TestRequest.php b/tests/Requests/TestRequest.php index f85333e..b42a93c 100644 --- a/tests/Requests/TestRequest.php +++ b/tests/Requests/TestRequest.php @@ -2,6 +2,9 @@ namespace Wedo\Api\Tests\Requests; +use Nette\Forms\Form; +use Wedo\Api\Attributes\Control; +use Wedo\Api\Attributes\ValidationRule; use Wedo\Api\Requests\BaseRequest; class TestRequest extends BaseRequest @@ -9,50 +12,43 @@ class TestRequest extends BaseRequest /** * Customer name - * - * @control Text - * @addRule(max_length, NULL, 20) - * @setRequired */ + #[Control(Control::TEXT)] + #[ValidationRule(Form::MAX_LENGTH, null, 20)] + #[ValidationRule(Form::REQUIRED)] public string $name; /** * Customers E-mail - * - * @control Email - * @addRule(min_length, NULL, 5) */ + #[Control(Control::EMAIL)] + #[ValidationRule(Form::MIN_LENGTH, null, 5)] public string $email; /** * Customers phone - * - * @addCustomRule('Wedo\Api\Tests\Requests\CustomValidator::validate', 'phone not valid') - * @control Text */ + #[Control(Control::TEXT)] + #[ValidationRule([CustomValidator::class, 'validate'])] public string $phone; /** * Customers birthYear - * - * @control Text - * @addRule(numeric) */ + #[Control(Control::TEXT)] + #[ValidationRule(Form::NUMERIC)] public ?int $birth_year; /** - * Customers birthYear - * - * @control Text - * @isRequired() + * Amount */ + #[Control(Control::TEXT)] public float $amount; /** * Accept tos - * - * @control CheckBox */ + #[Control(Control::CHECKBOX)] public bool $accept_tos; } diff --git a/tests/Requests/TestWithItemsOnTextRequest.php b/tests/Requests/TestWithItemsOnTextRequest.php deleted file mode 100644 index 5737d03..0000000 --- a/tests/Requests/TestWithItemsOnTextRequest.php +++ /dev/null @@ -1,16 +0,0 @@ - Date: Fri, 14 May 2021 08:29:27 +0200 Subject: [PATCH 4/4] drop nette/reflection --- composer.json | 1 - src/Helpers/FormBuilder.php | 10 ++++------ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/composer.json b/composer.json index e525539..409e516 100644 --- a/composer.json +++ b/composer.json @@ -8,7 +8,6 @@ "nette/di": "^3.0", "nette/application": "^3.0", "nette/forms": "^3.1.2", - "nette/reflection": "^2.4", "wedo/utilities": "^2.0" }, "require-dev": { diff --git a/src/Helpers/FormBuilder.php b/src/Helpers/FormBuilder.php index bd81e2d..bc5c68c 100644 --- a/src/Helpers/FormBuilder.php +++ b/src/Helpers/FormBuilder.php @@ -74,13 +74,11 @@ public function createForm(array $properties, BaseRequest $request, Container $f if ($control instanceof ChoiceControl) { $itemsAttributes = $property->getAttributes(ChoiceControlItems::class); - if (count($itemsAttributes) === 0) { - throw new NotSupportedException('Choice control must have ChoiceControlItems attribute set!'); + if (count($itemsAttributes) > 0) { + /** @var ChoiceControlItems $itemAttribute */ + $itemAttribute = $itemsAttributes[0]->newInstance(); + $control->setItems($itemAttribute->items, $itemAttribute->useKeys); } - - /** @var ChoiceControlItems $itemAttribute */ - $itemAttribute = $itemsAttributes[0]->newInstance(); - $control->setItems($itemAttribute->items, $itemAttribute->useKeys); } } }