Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TASK: Use deepObject for all non trivial parameters and remove [] from collection names #18

Merged
merged 3 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Classes/Domain/Metadata/Parameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,16 @@ public static function fromReflectionParameter(\ReflectionParameter $reflectionP
$parameterAttributes = $reflectionParameter->getAttributes(self::class);
switch (count($parameterAttributes)) {
case 0:
return new self(ParameterLocation::LOCATION_QUERY);
return new self(
in: ParameterLocation::LOCATION_QUERY,
style: ParameterStyle::createDefaultForParameterLocationAndReflection(ParameterLocation::LOCATION_QUERY, $reflectionParameter)
);
case 1:
$arguments = $parameterAttributes[0]->getArguments();

return new self(
$arguments['in'] ?? $arguments[0],
$arguments['style'] ?? $arguments[1] ?? null,
$arguments['style'] ?? $arguments[1] ?? ParameterStyle::createDefaultForParameterLocationAndReflection($arguments[0], $reflectionParameter),
$arguments['description'] ?? $arguments[2] ?? null,
);
default:
Expand Down
2 changes: 1 addition & 1 deletion Classes/Domain/Path/OpenApiParameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public static function fromReflectionParameter(\ReflectionParameter $reflectionP
$parameterSchema = OpenApiSchema::fromReflectionClass($reflectionClass);

return new self(
name: $reflectionParameter->name . (($parameterAttribute->in === ParameterLocation::LOCATION_QUERY && $parameterSchema->type === 'array') ? '[]' : ''),
name: $reflectionParameter->name,
in: $parameterAttribute->in,
description: $parameterAttribute->description ?: $schemaAttribute->description,
required: !$reflectionParameter->isDefaultValueAvailable(),
Expand Down
45 changes: 45 additions & 0 deletions Classes/Domain/Path/ParameterStyle.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

namespace Sitegeist\SchemeOnYou\Domain\Path;

use Sitegeist\SchemeOnYou\Domain\Schema\IsSingleValueDataTransferObject;
use Sitegeist\SchemeOnYou\Domain\Schema\OpenApiSchema;

/**
* @see https://swagger.io/specification/#style-values
*/
Expand All @@ -30,6 +33,48 @@ public static function createDefaultForParameterLocation(ParameterLocation $loca
};
}

/**
* @see https://swagger.io/specification/#fixed-fields-10
*/
public static function createDefaultForParameterLocationAndReflection(ParameterLocation $location, \ReflectionParameter $reflectionParameter): self
{
return match ($location) {
ParameterLocation::LOCATION_QUERY => self::createDefaultForQueryLocationAndReflection($reflectionParameter),
ParameterLocation::LOCATION_PATH => ParameterStyle::STYLE_SIMPLE,
ParameterLocation::LOCATION_HEADER => ParameterStyle::STYLE_SIMPLE,
ParameterLocation::LOCATION_COOKIE => ParameterStyle::STYLE_FORM
};
}

public static function createDefaultForQueryLocationAndReflection(\ReflectionParameter $reflectionParameter): self
{
$reflectionType = $reflectionParameter->getType();
if (!$reflectionType instanceof \ReflectionNamedType) {
throw new \DomainException(
'Query Parameters can only be resolved from named parameters',
1710067045
);
}
$type = $reflectionType->getName();
if (in_array($type, ['int', 'bool', 'string', 'float', \DateTimeImmutable::class, \DateTime::class, \DateInterval::class])) {
return ParameterStyle::STYLE_FORM;
}
if (!class_exists($type)) {
throw new \DomainException(
'Query parameters can only be resolved from class parameters, ' . $type . ' given for parameter '
. $reflectionParameter->getDeclaringClass()?->name
. '::' . $reflectionParameter->getDeclaringFunction()->name
. '::' . $reflectionParameter->name,
1709592649
);
}
if (IsSingleValueDataTransferObject::isSatisfiedByClassName($type)) {
return ParameterStyle::STYLE_FORM;
}

return ParameterStyle::STYLE_DEEP_OBJECT;
}

/**
* @todo really?
* @param array<mixed>|int|bool|string|float|null $parameterValue
Expand Down
5 changes: 3 additions & 2 deletions Classes/Domain/Schema/IsDataTransferObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ public static function isSatisfiedByReflectionClass(\ReflectionClass $reflection
*/
private static function evaluateReflectionClass(\ReflectionClass $reflectionClass): bool
{
if ($reflectionClass instanceof \ReflectionEnum) {
return true;
if ($reflectionClass->isEnum()) {
/** @phpstan-ignore-next-line */
return (new \ReflectionEnum($reflectionClass->getName()))->isBacked();
}
if ($reflectionClass->isReadOnly() === false) {
return false;
Expand Down
18 changes: 12 additions & 6 deletions Classes/Domain/Schema/IsDataTransferObjectCollection.php
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,13 @@ public static function isSatisfiedByReflectionClass(\ReflectionClass $reflection
*/
private static function evaluateReflectionClass(\ReflectionClass $reflectionClass): bool
{
$parameters = $reflectionClass->getConstructor()?->getParameters() ?: [];
if (count($parameters) !== 1) {
// readonly
if ($reflectionClass->isReadOnly() === false) {
return false;
}
if ($reflectionClass->isReadOnly() === false) {
// a single variadic constructor parameter of a supported type
$parameters = $reflectionClass->getConstructor()?->getParameters() ?: [];
if (count($parameters) !== 1) {
return false;
}
$collectionParameter = $parameters[0];
Expand All @@ -57,11 +59,15 @@ private static function evaluateReflectionClass(\ReflectionClass $reflectionClas
}
$collectionParameterType = $collectionParameter->getType();
if ($collectionParameterType instanceof \ReflectionNamedType) {
if (IsSupportedInSchema::isSatisfiedByReflectionType($collectionParameterType)) {
return true;
if (!IsSupportedInSchema::isSatisfiedByReflectionType($collectionParameterType)) {
return false;
}
}

// a single property of type array
$properties = $reflectionClass->getProperties();
if (count($properties) === 1 && $properties[0]->getType() instanceof \ReflectionNamedType && $properties[0]->getType()->getName() === 'array') {
return true;
}
return false;
}
}
58 changes: 58 additions & 0 deletions Classes/Domain/Schema/IsSingleValueDataTransferObject.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Domain\Schema;

use Neos\Flow\Annotations as Flow;

#[Flow\Proxy(false)]
final class IsSingleValueDataTransferObject
{
/**
* @var array<class-string,bool>
*/
private static array $evaluationRuntimeCache = [];

/**
* @param class-string $className
*/
public static function isSatisfiedByClassName(string $className): bool
{
if (!array_key_exists($className, self::$evaluationRuntimeCache)) {
self::$evaluationRuntimeCache[$className] = self::evaluateReflectionClass(new \ReflectionClass($className));
}

return self::$evaluationRuntimeCache[$className];
}

/**
* @param \ReflectionClass<object> $reflectionClass
*/
public static function isSatisfiedByReflectionClass(\ReflectionClass $reflectionClass): bool
{
if (!array_key_exists($reflectionClass->name, self::$evaluationRuntimeCache)) {
self::$evaluationRuntimeCache[$reflectionClass->name] = self::evaluateReflectionClass($reflectionClass);
}

return self::$evaluationRuntimeCache[$reflectionClass->name];
}

/**
* @param \ReflectionClass<object> $reflectionClass
*/
private static function evaluateReflectionClass(\ReflectionClass $reflectionClass): bool
{
if ($reflectionClass->isEnum()) {
/** @phpstan-ignore-next-line */
return (new \ReflectionEnum($reflectionClass->getName()))->isBacked();
}
if (IsDataTransferObject::isSatisfiedByReflectionClass($reflectionClass)) {
$parameters = $reflectionClass->getConstructor()?->getParameters() ?: [];
if (count($parameters) === 1 && $parameters[0]->name === 'value') {
return true;
}
}
return false;
}
}
22 changes: 22 additions & 0 deletions Tests/Fixtures/InvalidObjects/CollectionThatIsNotReadonly.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Tests\Fixtures\InvalidObjects;

use Neos\Flow\Annotations as Flow;
use Sitegeist\SchemeOnYou\Tests\Fixtures;

#[Flow\Proxy(false)]
final class CollectionThatIsNotReadonly
{
/**
* @var Fixtures\Identifier[]
*/
public array $items;
public function __construct(
Fixtures\Identifier ...$items
) {
$this->items = $items;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Tests\Fixtures\InvalidObjects;

use Neos\Flow\Annotations as Flow;
use Sitegeist\SchemeOnYou\Tests\Fixtures;

#[Flow\Proxy(false)]
final readonly class CollectionWithMoreThanOneProperty
{
/**
* @var Fixtures\Identifier[]
*/
public array $items;

public int $count;

public function __construct(
Fixtures\Identifier ...$items
) {
$this->items = $items;
$this->count = count($items);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Tests\Fixtures\InvalidObjects;

use Neos\Flow\Annotations as Flow;
use Sitegeist\SchemeOnYou\Tests\Fixtures;

#[Flow\Proxy(false)]
final readonly class CollectionWithTooManyConstructorArguments
{
/**
* @var Fixtures\Identifier[]
*/
public array $items;
public function __construct(
string $other,
Fixtures\Identifier ...$items
) {
if ($other === 'whatever') {
$this->items = [...$items];
} else {
$this->items = $items;
}
}
}
14 changes: 14 additions & 0 deletions Tests/Fixtures/InvalidObjects/NonBackedEnum.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Tests\Fixtures\InvalidObjects;

use Neos\Flow\Annotations as Flow;

#[Flow\Proxy(false)]
enum NonBackedEnum
{
case Foo;
case Bar;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Tests\Fixtures\InvalidObjects;

use Neos\Flow\Annotations as Flow;
use Sitegeist\SchemeOnYou\Domain\Metadata as OpenApi;

#[Flow\Proxy(false)]
final readonly class ObjectThatHasNonPromotedConstructorArguments
{
public string $text;
public int $num;
public bool $switch;

public function __construct(
string $text,
int $num,
bool $switch
) {

$this->text = $text;
$this->num = $num;
$this->switch = $switch;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Tests\Fixtures\InvalidObjects;

use Neos\Flow\Annotations as Flow;

#[Flow\Proxy(false)]
final readonly class ObjectThatHasNotSupportedProperties
{
public function __construct(
public \Psr\Http\Message\RequestInterface $text
) {
}
}
18 changes: 18 additions & 0 deletions Tests/Fixtures/InvalidObjects/ObjectThatIsNotReadonly.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Sitegeist\SchemeOnYou\Tests\Fixtures\InvalidObjects;

use Neos\Flow\Annotations as Flow;

#[Flow\Proxy(false)]
final class ObjectThatIsNotReadonly
{
public function __construct(
public string $text,
public int $num,
public bool $switch
) {
}
}
24 changes: 13 additions & 11 deletions Tests/Unit/Application/ParameterFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,12 @@ public static function parameterProvider(): iterable
'request' => ActionRequest::fromHttpRequest(
(new ServerRequest(
HttpMethod::METHOD_GET->value,
new Uri('https://acme.site/?endpointQuery=de')
))->withQueryParams([
'endpointQuery' => [
'language' => 'de'
new Uri('https://acme.site/')
))->withQueryParams(
[
'endpointQuery' => '{"language":"de"}'
]
])
)
),
'className' => PathController::class,
'methodName' => 'singleParameterAndResponseEndpointAction',
Expand Down Expand Up @@ -97,10 +97,12 @@ public static function parameterProvider(): iterable
$multipleParametersRequest = ActionRequest::fromHttpRequest(
(new ServerRequest(
HttpMethod::METHOD_GET->value,
new Uri('https://acme.site/?endpointQuery=de')
))->withQueryParams([
'anotherEndpointQuery' => '{"pleaseFail": true}'
])
new Uri('https://acme.site/de/')
))->withQueryParams(
[
'anotherEndpointQuery' => '{"pleaseFail":"true"}'
]
)
);
$multipleParametersRequest->setArgument('endpointQuery', ['language' => 'de']);

Expand All @@ -117,9 +119,9 @@ public static function parameterProvider(): iterable
$collectionParametersRequest = ActionRequest::fromHttpRequest(
(new ServerRequest(
HttpMethod::METHOD_GET->value,
new Uri('https://acme.site/?identifierCollection[]=foo&identifierCollection[]=bar&identifier=baz')
new Uri('https://acme.site/?identifierCollection=foo&identifierCollection=bar&identifier=baz')
))->withQueryParams([
'identifierCollection' => ['foo','bar'],
'identifierCollection' => '["foo","bar"]',
'identifier' => 'baz'
])
);
Expand Down
Loading
Loading