From b72b57692e2d6c7bc74cef93b3298791fe1af8ba Mon Sep 17 00:00:00 2001 From: Dalibor Korpar Date: Fri, 7 May 2021 11:19:06 +0200 Subject: [PATCH] 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 | 5 +- 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, 94 insertions(+), 132 deletions(-) create mode 100644 src/Attributes/Internal.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 index ee0e47d..cf957b7 100644 --- a/src/Helpers/AnnotationHelper.php +++ b/src/Helpers/AnnotationHelper.php @@ -11,12 +11,11 @@ class AnnotationHelper /** * Returns an annotation value. - * - * @return string|IAnnotation */ - public static function getAnnotation(Reflector $reflector, string $name) + public static function getAnnotation(Reflector $reflector, string $name): string|IAnnotation|null { $res = AnnotationsParser::getAll($reflector); + return isset($res[$name]) ? end($res[$name]) : null; } diff --git a/src/Helpers/FormBuilder.php b/src/Helpers/FormBuilder.php index 1ff43cd..09e8ce1 100644 --- a/src/Helpers/FormBuilder.php +++ b/src/Helpers/FormBuilder.php @@ -65,6 +65,11 @@ public function createForm(array $properties, BaseRequest $request, Container $f } } + public function createEmptyForm(): Form + { + return new Form(); + } + /** * @param string[][] $annotations * @throws ArgumentOutOfRangeException @@ -79,9 +84,4 @@ protected function getControlType(array $annotations): string return $annotations['control'][0]; } - public function createEmptyForm(): Form - { - return new Form(); - } - } diff --git a/src/Helpers/RequestHydrator.php b/src/Helpers/RequestHydrator.php index ba83091..b1b9b8f 100644 --- a/src/Helpers/RequestHydrator.php +++ b/src/Helpers/RequestHydrator.php @@ -9,11 +9,7 @@ class RequestHydrator { - /** - * @param mixed|string $value - * @return string|int|float|bool|DateTimeImmutable|null - */ - public static function hydrateValue(ReflectionNamedType $type, $value) + public static function hydrateValue(ReflectionNamedType $type, mixed $value): string|int|float|bool|DateTimeImmutable|null { if (!$type->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); }