Skip to content

Commit

Permalink
style: various code smells
Browse files Browse the repository at this point in the history
  • Loading branch information
soyuka committed Sep 14, 2022
1 parent bbba5d8 commit ac46ebd
Show file tree
Hide file tree
Showing 13 changed files with 26 additions and 28 deletions.
2 changes: 1 addition & 1 deletion features/graphql/authorization.feature
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ Feature: Authorization checking
"""
Then the response status code should be 200
And the response should be in JSON
And the JSON node "data.securedDummy.ownerOnlyProperty" should be equal to the string ""
And the JSON node "data.securedDummy.ownerOnlyProperty" should be equal to ""

Scenario: A user cannot retrieve an item they doesn't own
When I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
Expand Down
2 changes: 1 addition & 1 deletion features/main/custom_operation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Feature: Custom operation

@createSchema
Scenario: Void a payment
Given There is a payment
Given there is a payment
When I send a "POST" request to "/payments/1/void"
Then the response status code should be 201
And the response should be in JSON
Expand Down
9 changes: 5 additions & 4 deletions src/Action/ExceptionAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,13 @@ private function getOperationExceptionToStatus(Request $request): array
$resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($attributes['resource_class']);
/** @var HttpOperation $operation */
$operation = $resourceMetadataCollection->getOperation($attributes['operation_name'] ?? null);
$exceptionToStatus = $operation->getExceptionToStatus() ?: [];
$exceptionToStatus = [$operation->getExceptionToStatus() ?: []];

foreach ($resourceMetadataCollection as $resourceMetadata) {
/** @var ApiResource $resourceMetadata */
$exceptionToStatus = array_merge($exceptionToStatus, $resourceMetadata->getExceptionToStatus() ?: []);
/* @var ApiResource $resourceMetadata */
$exceptionToStatus[] = $resourceMetadata->getExceptionToStatus() ?: [];
}

return $exceptionToStatus;
return array_merge(...$exceptionToStatus);
}
}
3 changes: 2 additions & 1 deletion src/Api/IdentifiersExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ public function getIdentifiersFromItem(object $item, Operation $operation = null
continue;
}

$identifiers[$link->getParameterName()] = $this->getIdentifierValue($item, $link->getFromClass(), $link->getIdentifiers()[0], $link->getParameterName());
$parameterName = $link->getParameterName();
$identifiers[$parameterName] = $this->getIdentifierValue($item, $link->getFromClass(), $link->getIdentifiers()[0], $parameterName);
}

return $identifiers;
Expand Down
6 changes: 4 additions & 2 deletions src/Api/QueryParameterValidator/QueryParameterValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,15 @@ public function validateFilters(string $resourceClass, array $resourceFilters, a

foreach ($filter->getDescription($resourceClass) as $name => $data) {
foreach ($this->validators as $validator) {
$errorList = array_merge($errorList, $validator->validate($name, $data, $queryParameters));
if ($errors = $validator->validate($name, $data, $queryParameters)) {
$errorList[] = $errors;
}
}
}
}

if ($errorList) {
throw new FilterValidationException($errorList);
throw new FilterValidationException(array_merge(...$errorList));
}
}
}
6 changes: 3 additions & 3 deletions src/Doctrine/Common/State/LinksHandlerTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ private function getLinks(string $resourceClass, Operation $operation, array $co

// Instead, we'll look for the first Query available.
foreach ($resourceMetadataCollection as $resourceMetadata) {
foreach ($resourceMetadata->getGraphQlOperations() as $operation) {
if ($operation instanceof Query) {
$linkedOperation = $operation;
foreach ($resourceMetadata->getGraphQlOperations() as $op) {
if ($op instanceof Query) {
$linkedOperation = $op;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/GraphQl/Action/EntrypointAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ private function applyMapToVariables(array $map, array $variables, array $files)
throw new BadRequestHttpException('GraphQL multipart request file has not been sent correctly.');
}

foreach ($map[$key] as $mapValue) {
foreach ($value as $mapValue) {
$path = explode('.', (string) $mapValue);

if ('variables' !== $path[0]) {
Expand Down
2 changes: 1 addition & 1 deletion src/GraphQl/Serializer/SerializerContextBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ private function replaceIdKeys(array $fields, ?string $resourceClass, array $con
continue;
}

$denormalizedFields[$this->denormalizePropertyName((string) $key, $resourceClass, $context)] = \is_array($fields[$key]) ? $this->replaceIdKeys($fields[$key], $resourceClass, $context) : $value;
$denormalizedFields[$this->denormalizePropertyName((string) $key, $resourceClass, $context)] = \is_array($value) ? $this->replaceIdKeys($value, $resourceClass, $context) : $value;
}

return $denormalizedFields;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,9 @@ private function getClassSerializerGroups(string $class): array

$groups = [];
foreach ($serializerClassMetadata->getAttributesMetadata() as $serializerAttributeMetadata) {
$groups = array_merge($groups, $serializerAttributeMetadata->getGroups());
$groups[] = $serializerAttributeMetadata->getGroups();
}

return array_unique($groups);
return array_unique(array_merge(...$groups));
}
}
2 changes: 1 addition & 1 deletion src/Serializer/AbstractItemNormalizer.php
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ protected function instantiateObject(array &$data, string $class, array &$contex
throw new RuntimeException(sprintf('Cannot create an instance of %s from serialized data because the variadic parameter %s can only accept an array.', $class, $constructorParameter->name));
}

$params = array_merge($params, $data[$paramName]);
$params[] = $data[$paramName];
}
} elseif ($allowed && !$ignored && (isset($data[$key]) || \array_key_exists($key, $data))) {
$params[] = $this->createConstructorArgument($data[$key], $key, $constructorParameter, $context, $format);
Expand Down
6 changes: 2 additions & 4 deletions src/Symfony/Routing/ApiLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,8 @@ public function load(mixed $data, string $type = null): RouteCollection
$path = str_replace(sprintf('{%s}', $parameterName), $expandedValue, $path);
}

if ($controller = $operation->getController()) {
if (!$this->container->has($controller)) {
throw new RuntimeException(sprintf('There is no builtin action for the "%s" operation. You need to define the controller yourself.', $operationName));
}
if (($controller = $operation->getController()) && !$this->container->has($controller)) {
throw new RuntimeException(sprintf('There is no builtin action for the "%s" operation. You need to define the controller yourself.', $operationName));
}

$route = new Route(
Expand Down
8 changes: 3 additions & 5 deletions tests/Fixtures/TestBundle/Entity/SubresourceOrganization.php
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,9 @@ public function addSubresourceEmployee(SubresourceEmployee $employee): self

public function removeSubresourceEmployee(SubresourceEmployee $employee): self
{
if ($this->employees->removeElement($employee)) {
// set the owning side to null (unless already changed)
if ($employee->getSubresourceOrganization() === $this) {
$employee->setSubresourceOrganization(null);
}
// set the owning side to null (unless already changed)
if ($this->employees->removeElement($employee) && $employee->getSubresourceOrganization() === $this) {
$employee->setSubresourceOrganization(null);
}

return $this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ class UploadMultipleMediaObjectResolver implements MutationResolverInterface
{
public function __invoke(?object $item, array $context): MediaObject
{
$result = [];
$mediaObject = null;

/**
Expand All @@ -40,7 +39,6 @@ public function __invoke(?object $item, array $context): MediaObject
$mediaObject = new MediaObject();
$mediaObject->id = $key;
$mediaObject->contentUrl = $uploadedFile->getFilename();
$result[] = $mediaObject;
}

// Currently API Platform does not support custom mutation with collections so for now, we are returning the last created media object.
Expand Down

0 comments on commit ac46ebd

Please sign in to comment.