Skip to content

Commit

Permalink
Merge 4db5b25 into a3b833f
Browse files Browse the repository at this point in the history
  • Loading branch information
soyuka committed Dec 16, 2021
2 parents a3b833f + 4db5b25 commit 074d2c6
Show file tree
Hide file tree
Showing 96 changed files with 3,224 additions and 924 deletions.
3 changes: 1 addition & 2 deletions .github/workflows/ci.yml
Expand Up @@ -847,7 +847,7 @@ jobs:
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
restore-keys: ${{ runner.os }}-composer-
- name: Require Symfony components and Rector dependencies
run: composer require symfony/intl symfony/uid rector/rector --dev --no-interaction --no-progress --ansi
run: composer require symfony/intl symfony/uid rector/rector --dev --no-interaction --no-progress --ansi --ignore-platform-reqs
- name: Install PHPUnit
run: vendor/bin/simple-phpunit --version
- name: Clear test app cache
Expand All @@ -864,7 +864,6 @@ jobs:
run: |
mkdir -p build/logs/behat
vendor/bin/behat --out=std --format=progress --format=junit --out=build/logs/behat/junit --profile=default --no-interaction
continue-on-error: true
- name: Upload test artifacts
if: always()
uses: actions/upload-artifact@v1
Expand Down
8 changes: 4 additions & 4 deletions docs/adr/0003-uri-variables.md
Expand Up @@ -37,7 +37,7 @@ We will use a POPO to define URI variables, for now these options are available:
uriVariables: [
'companyId' => new UriVariable(
targetClass: Company::class,
inverseProperty: null,
targetProperty: null,
property: 'company'
identifiers: ['id'],
compositeIdentifier: true,
Expand All @@ -53,7 +53,7 @@ Where `uriVariables` keys are the URI template's variable names. Its value is a

- `targetClass` is the PHP FQDN of the class this value belongs to
- `property` represents the property, the URI Variable is mapped to in the current class
- `inverseProperty` represents the property, the URI Variable is mapped to in the related class and is not available in the current class
- `targetProperty` represents the property, the URI Variable is mapped to in the related class and is not available in the current class
- `identifiers` are the properties of the targetClass to which we map the URI variable
- `compositeIdentifier` is used to match a single variable to multiple identifiers (`ida=1;idb=2` to `class::ida` and `class::idb`)

Expand Down Expand Up @@ -122,7 +122,7 @@ class Company {
}
```

Note that the above is a shortcut for: `new UriVariable(targetClass: Employee::class, inverseProperty: 'company')`
Note that the above is a shortcut for: `new UriVariable(targetClass: Employee::class, targetProperty: 'company')`

Corresponding DQL:

Expand Down Expand Up @@ -259,7 +259,7 @@ class Employee {
#[ApiResource("/employees/{employeeId}/company", uriVariables: [
'employeeId' => new UriVariable(
targetClass: Employee::class,
inverseProperty: 'company'
targetProperty: 'company'
property: null,
identifiers: ['id'],
compositeIdentifier: true
Expand Down
146 changes: 146 additions & 0 deletions docs/adr/0004-link.md
@@ -0,0 +1,146 @@
# Link

* Status: accepted
* Deciders: @dunglas, @soyuka, @alanpoulain

Implementation: [#4536][pull/4536]

## Context and Problem Statement

The [URI Variables](0003-uri-variables.md) ADR introduces a new `UriVariable` POPO.
In GraphQL, having URI variables make no sense: this object needs either an alias or needs to be named differently.

## Considered Options

* Create a `Traverser` alias for GraphQL.
* Rename `UriVariable` to `Link`.

## Decision Outcome

We chose to rename `UriVariable` to `Link` in order to simplify the codebase.
However the `uriVariables` parameter in the REST operations will not be renamed since it makes sense to have this name.
GraphQL operations don't need to have links at the operation level, a `Link` attribute on the property will be used instead if necessary (the main use case is when a `toProperty` is necessary).

To follow this renaming, the properties in `Link` are also renamed:
- `targetClass` becomes `fromClass`
- `inverseProperty` becomes `fromProperty`
- `property` becomes `toProperty`

New properties are also necessary:
- `toClass` for GraphQL because GraphQL needs to find the right `Link`
- `expandedValue` for REST in order to convert an URI variable to the corresponding route part (for instance in the case of the URI template `/questions/{questionId}/{questionAnswer}/related_questions`, the expanded value for `questionAnswer` could be `answer`)

### Classical Example

```php
<?php

#[Query]
#[Get]
class Company
{
public $id;

#[ORM\OneToMany(targetEntity: Employee::class, mappedBy: 'company')]
// will automatically create:
#[Link(fromClass: Company::class, fromProperty: 'employees')]
/** @var Employee[] */
public iterable $employees;
}
```

```php
<?php

#[Query]
#[GetCollection('/companies/{companyId}/employees', uriVariables: [
'companyId' => new Link(
fromClass: Company::class,
fromProperty: 'employees'
)
])]
class Employee
{
public $id;

#[ORM\ManyToOne(targetEntity: Company::class, inversedBy: 'employees')]
public Company $company;
}
```

The GraphQL query equivalent to a `GET` to `/companies/2/employees` can now be done:

```graphql
{
companies(id: "/companies/2") {
employees {
edges {
node {
id
}
}
}
}
}
```

### Inverted Example

In this example, the relation between the employee and the company is only hold by the employee.

```php
<?php

#[Query]
#[GetCollection('/companies/{companyId}/employees', uriVariables: [
'companyId' => new Link(
fromClass: Company::class,
toProperty: 'company'
)
])]
class Employee
{
public $id;

#[ORM\ManyToMany(targetEntity: Company::class)]
#[ORM\JoinTable(name: 'employees_companies')]
#[ORM\JoinColumn(name: 'employee_id', referencedColumnName: 'id')]
#[ORM\InverseJoinColumn(name: 'company_id', referencedColumnName: 'id', unique: true)]
public Company $company;
}
```

```php
<?php

#[Query]
#[Get]
class Company
{
public $id;

#[Link('company')]
// equivalent to:
#[Link(fromClass: Company::class, toClass: Employee::class, toProperty: 'company')]
/** @var Employee[] */
public iterable $employees;
}
```

The GraphQL query equivalent to a `GET` to `/companies/2/employees` can now be done:

```graphql
{
companies(id: "/companies/2") {
employees {
edges {
node {
id
}
}
}
}
}
```

[pull/4536]: https://github.com/api-platform/core/pull/4536 "Link implementation"
4 changes: 2 additions & 2 deletions features/doctrine/eager_loading.feature
Expand Up @@ -17,7 +17,7 @@ Feature: Eager Loading
LEFT JOIN thirdLevel_a1.fourthLevel fourthLevel_a2
LEFT JOIN o.relatedToDummyFriend relatedToDummyFriend_a3
LEFT JOIN relatedToDummyFriend_a3.dummyFriend dummyFriend_a4
WHERE o.id = :id_id
WHERE o.id = :id_p1
"""

Scenario: Eager loading for the search filter
Expand Down Expand Up @@ -74,7 +74,7 @@ Feature: Eager Loading
FROM ApiPlatform\Tests\Fixtures\TestBundle\Entity\DummyTravel o
LEFT JOIN o.car car_a1
LEFT JOIN o.passenger passenger_a2
WHERE o.id = :id_id
WHERE o.id = :id_p1
"""

Scenario: Eager loading for a relation with complex sub-query filter
Expand Down
2 changes: 1 addition & 1 deletion features/jsonld/input_output.feature
Expand Up @@ -282,7 +282,7 @@ Feature: JSON-LD DTO input and output

@!mongodb
Scenario: Use messenger with an input where the handler gives a synchronous result
And I send a "POST" request to "/messenger_with_inputs" with body:
When I send a "POST" request to "/messenger_with_inputs" with body:
"""
{
"var": "test"
Expand Down
1 change: 1 addition & 0 deletions features/jsonld/non_resource.feature
Expand Up @@ -7,6 +7,7 @@ Feature: JSON-LD non-resource handling
Given I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"

@createSchema
Scenario: Get a resource containing a raw object
When I send a "GET" request to "/contain_non_resources/1"
Then the response status code should be 200
Expand Down
29 changes: 29 additions & 0 deletions features/main/crud_uri_variables.feature
Expand Up @@ -121,6 +121,35 @@ Feature: Uri Variables
"hydra:totalItems": 2
}
"""
When I send the following GraphQL request:
"""
{
companies {
edges {
node {
name
employees {
edges {
node {
name
}
}
}
}
}
}
}
"""
Then the response status code should be 200
And the response should be in JSON
And the header "Content-Type" should be equal to "application/json"
And the JSON node "data.companies.edges[0].node.name" should be equal to "Foo Company 1"
And the JSON node "data.companies.edges[0].node.employees.edges" should have 1 element
And the JSON node "data.companies.edges[0].node.employees.edges[0].node.name" should be equal to "foo"
And the JSON node "data.companies.edges[1].node.name" should be equal to "Foo Company 2"
And the JSON node "data.companies.edges[1].node.employees.edges" should have 2 elements
And the JSON node "data.companies.edges[1].node.employees.edges[0].node.name" should be equal to "foo2"
And the JSON node "data.companies.edges[1].node.employees.edges[1].node.name" should be equal to "foo3"

@php8
Scenario: Retrieve the company of an employee
Expand Down
3 changes: 3 additions & 0 deletions features/main/subresource.feature
Expand Up @@ -223,7 +223,9 @@ Feature: Subresource support
}
"""

@createSchema
Scenario: Get the subresource relation item
Given there is a dummy object with a fourth level relation
When I send a "GET" request to "/dummies/1/related_dummies/2"
Then the response status code should be 200
And the response should be in JSON
Expand Down Expand Up @@ -299,6 +301,7 @@ Feature: Subresource support
}
"""

@createSchema
Scenario: Get offers subresource from aggregate offers subresource
Given I have a product with offers
When I send a "GET" request to "/dummy_products/2/offers/1/offers"
Expand Down
25 changes: 25 additions & 0 deletions rector.patch
@@ -0,0 +1,25 @@
diff --git a/composer.json b/composer.json
index 92a283f08..ccbff9ba1 100644
--- a/composer.json
+++ b/composer.json
@@ -43,15 +43,15 @@
"friends-of-behat/mink-extension": "^2.2",
"friends-of-behat/symfony-extension": "^2.1",
"guzzlehttp/guzzle": "^6.0 || ^7.0",
- "jangregor/phpstan-prophecy": "^0.8",
+ "jangregor/phpstan-prophecy": "*",
"justinrainbow/json-schema": "^5.2.1",
"phpdocumentor/reflection-docblock": "^3.0 || ^4.0 || ^5.1",
"phpdocumentor/type-resolver": "^0.3 || ^0.4 || ^1.4",
"phpstan/extension-installer": "^1.0",
- "phpstan/phpstan": "^0.12.65",
- "phpstan/phpstan-doctrine": "^0.12.7",
- "phpstan/phpstan-phpunit": "^0.12.4",
- "phpstan/phpstan-symfony": "^0.12.4",
+ "phpstan/phpstan": "*",
+ "phpstan/phpstan-doctrine": "*",
+ "phpstan/phpstan-phpunit": "*",
+ "phpstan/phpstan-symfony": "*",
"psr/log": "^1.0",
"ramsey/uuid": "^3.7 || ^4.0",
"ramsey/uuid-doctrine": "^1.4",
28 changes: 15 additions & 13 deletions src/Api/IdentifiersExtractor.php
Expand Up @@ -16,6 +16,7 @@
use ApiPlatform\Core\Api\ResourceClassResolverInterface;
use ApiPlatform\Core\Identifier\CompositeIdentifierParser;
use ApiPlatform\Exception\RuntimeException;
use ApiPlatform\Metadata\GraphQl\Operation as GraphQlOperation;
use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
Expand Down Expand Up @@ -56,18 +57,19 @@ public function getIdentifiersFromItem($item, string $operationName = null, arra
$resourceClass = $this->getResourceClass($item, true);
$operation = $context['operation'] ?? $this->resourceMetadataFactory->create($resourceClass)->getOperation($operationName);

foreach ($operation->getUriVariables() ?? [] as $parameterName => $uriVariableDefinition) {
if (1 < \count($uriVariableDefinition->getIdentifiers())) {
$links = $operation instanceof GraphQlOperation ? $operation->getLinks() : $operation->getUriVariables();
foreach ($links ?? [] as $link) {
if (1 < \count($link->getIdentifiers())) {
$compositeIdentifiers = [];
foreach ($uriVariableDefinition->getIdentifiers() as $identifier) {
$compositeIdentifiers[$identifier] = $this->getIdentifierValue($item, $uriVariableDefinition->getTargetClass() ?? $resourceClass, $identifier, $parameterName);
foreach ($link->getIdentifiers() as $identifier) {
$compositeIdentifiers[$identifier] = $this->getIdentifierValue($item, $link->getFromClass() ?? $resourceClass, $identifier, $link->getParameterName());
}

$identifiers[($operation->getExtraProperties()['is_legacy_resource_metadata'] ?? false) ? 'id' : $parameterName] = CompositeIdentifierParser::stringify($compositeIdentifiers);
$identifiers[($operation->getExtraProperties()['is_legacy_resource_metadata'] ?? false) ? 'id' : $link->getParameterName()] = CompositeIdentifierParser::stringify($compositeIdentifiers);
continue;
}

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

return $identifiers;
Expand All @@ -92,13 +94,13 @@ private function getIdentifierValue($item, string $class, string $property, stri
continue;
}

if ($type->getClassName() === $class) {
return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, "$propertyName.$property"), $parameterName);
}

if ($type->isCollection() && ($collectionValueType = $type->getCollectionValueType()) && $collectionValueType->getClassName() === $class) {
return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, sprintf('%s[0].%s', $propertyName, $property)), $parameterName);
}

if ($type->getClassName() === $class) {
return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, "$propertyName.$property"), $parameterName);
}
}

throw new RuntimeException('Not able to retrieve identifiers.');
Expand Down Expand Up @@ -126,9 +128,9 @@ private function resolveIdentifierValue($identifierValue, string $parameterName)
if ($this->isResourceClass($relatedResourceClass = $this->getObjectClass($identifierValue))) {
trigger_deprecation('api-platform/core', '2.7', 'Using a resource class as identifier is deprecated, please make this identifier Stringable');
$relatedOperation = $this->resourceMetadataFactory->create($relatedResourceClass)->getOperation();
$relatedIdentifiers = $relatedOperation->getUriVariables();
if (1 === \count($relatedIdentifiers)) {
$identifierValue = $this->getIdentifierValue($identifierValue, $relatedResourceClass, current($relatedIdentifiers)->getIdentifiers()[0], $parameterName);
$relatedLinks = $relatedOperation instanceof GraphQlOperation ? $relatedOperation->getLinks() : $relatedOperation->getUriVariables();
if (1 === \count($relatedLinks)) {
$identifierValue = $this->getIdentifierValue($identifierValue, $relatedResourceClass, current($relatedLinks)->getIdentifiers()[0], $parameterName);

if ($identifierValue instanceof \Stringable || is_scalar($identifierValue) || method_exists($identifierValue, '__toString')) {
return (string) $identifierValue;
Expand Down

0 comments on commit 074d2c6

Please sign in to comment.