Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Add documentation about doctrine query extensions #120

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

Merged
dunglas merged 12 commits into api-platform:master from GregoireHebert:master
Oct 24, 2016

Conversation

@GregoireHebert
Copy link
Contributor

@GregoireHebert GregoireHebert commented Oct 7, 2016

No description provided.

Copy link
Member

soyuka commented Oct 7, 2016

see #107

Copy link
Member

dunglas commented Oct 7, 2016

@soyuka IIUC this PR doesn't cover the same topic than #107. Both should be merged.

Copy link
Member

soyuka commented Oct 7, 2016

+1 should've written related
dunglas reacted with thumbs up emoji

@@ -0,0 +1,164 @@
# Extensions

API Platform Core provides a system to extends queries on items and collections readings.
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extend(削除) s (削除ここまで) [...] (削除) readings (削除ここまで)

# Extensions

API Platform Core provides a system to extends queries on items and collections readings.
You can create custom extension that fit your needs.
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a custom [...]

Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sentence can be removed, it doesn't add something useful.


## Custom Extension

If Doctrine ORM support is enabled, adding extension is as easy as registering a service in your `app/config/services.yml` file and create the extension you need.
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding an extension
create the class?


If Doctrine ORM support is enabled, adding extension is as easy as registering a service in your `app/config/services.yml` file and create the extension you need.

Custom extension can be written by implementing the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface`
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A custom extension must implement ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface and/or ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryItemExtensionInterface interfaces


Custom extension can be written by implementing the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface`
and / or the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryItemExtensionInterface`
interfaces, depending on your whether you asking for a collection of items or just an item.
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depending if you are asking for

}

/**
* Add where condition
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adds the WHERE condition.

Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe that this docblock can be removed from the doc as it contains no useful information (typehints are enough).

}

$classMetadata = $queryBuilder->getEntityManager()->getClassMetadata($resourceClass);
$user = $this->token->getToken()->getUser();
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should test if $user is an instance of User because IIRC Symfony will return the string 'anon.' when the user isn't logged in.

Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively you can add a firewall rule to check that the user is connected.

$mapping = $classMetadata->associationMappings[$association];

if (User::class === $mapping['targetEntity']) {
$queryBuilder->andWhere('o.'.$association.' = '.$user->getId());
Copy link
Member

@dunglas dunglas Oct 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You must use parameters. This is a potential security issue.
Btw, why do you need to access to the class metadata? Can't you hardcode o.user? As it's a code you control I think it's ok in term of design and it will be less complex.

- '@security.token_storage'
- '@security.authorization_checker'
tags:
- { name: api_platform.doctrine.orm.query_extension.collection, priority: 64 }
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to add a priority? If it is necessary, it should be explained. If it's not, it can be removed.

- { name: api_platform.doctrine.orm.query_extension.item, priority: 64 }
```
Having the item related tag and interface, allows you to customize the query when trying to get/read a specific Item.
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say:

Thanks to the api_platform.doctrine.orm.query_extension.collection tag, API Platform will register this service as a collection extension. The api_platform.doctrine.orm.query_extension.item do the same thing for items.

$queryBuilder->andWhere('o.'.$association.' = '.$user->getId());
return;
}
if (Offer::class === $resourceClass && !$this->authorizationChecker->isGranted('ROLE_ADMIN')) {
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep the guard clause. It reduce the code complexity and improve its readability.

if (Offer::class === $resourceClass && !$this->authorizationChecker->isGranted('ROLE_ADMIN')) {
$user = $this->token->getToken()->getUser();
$rootAlias = $queryBuilder->getRootAliases()[0];
$queryBuilder->andWhere(sprintf('%s.user = ', $rootAlias).$user->getId());
Copy link
Member

@dunglas dunglas Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

teohhanhui reacted with thumbs up emoji
Copy link
Member

@dunglas dunglas Oct 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using parameters will also allow the DBMS to keep the prepared query in cache.

Copy link
Contributor Author

I made the changes, i'm ready to squash.

- { name: api_platform.doctrine.orm.query_extension.item }
```
Thanks to the api_platform.doctrine.orm.query_extension.collection tag, API Platform will register this service as a collection extension.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

`api_platform.doctrine.orm.query_extension.collection `

```
Thanks to the api_platform.doctrine.orm.query_extension.collection tag, API Platform will register this service as a collection extension.
The api_platform.doctrine.orm.query_extension.item do the same thing for items.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

`api_platform.doctrine.orm.query_extension.item`

Thanks to the api_platform.doctrine.orm.query_extension.collection tag, API Platform will register this service as a collection extension.
The api_platform.doctrine.orm.query_extension.item do the same thing for items.
Notice the priority level for the Collection tag.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

api_platform.doctrine.orm.query_extension.collection tag

The api_platform.doctrine.orm.query_extension.item do the same thing for items.
Notice the priority level for the Collection tag.
There is a case, when an extension implements the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryResultCollectionExtensionInterface` or the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryResultItemExtensionInterface` and supports to immediately return results,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"There is a case" can be removed

Copy link
Member

@dunglas dunglas Oct 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] interface to return results by itself,

Notice the priority level for the Collection tag.
There is a case, when an extension implements the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryResultCollectionExtensionInterface` or the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryResultItemExtensionInterface` and supports to immediately return results,
any lower priority extension will not be executed.
In our case, since the pagination is activated by default ([see how to disable the pagination](pagination.md#disabling-the-pagination)) and the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\PaginationExtension` is declared with a priority 8, we must declare a priority to at least 9 to ensure it's execution.
Copy link
Member

@dunglas dunglas Oct 12, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the pagination is enabled by default
a priority of 8
the priority of the app.doctrine.orm.query_extension.current_user service must be at least 9 to ensure its execution.

API Platform Core provides a system to extend queries on items and collections.

Extensions are specific to Doctrine, and therefor, the Doctrine ORM support must be enabled.
If you use custom providers, they should support extensions and be aware of active extensions OR implement their own extension systems.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say: If you use custom providers it's up to you to implement your own extension system or not.

If you use [custom data providers](data-providers.md), they must support extensions and be aware of active extensions to work
properly.

## Filter upon the current user
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change by Example

{
// ...
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra blank line to remove


//...
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra blank line to remove

private function addWhere(QueryBuilder $queryBuilder, string $resourceClass)
{
$user = $this->token->getToken()->getUser();
if ($user instanceof User && Offer::class === $resourceClass && !$this->authorizationChecker->isGranted('ROLE_ADMIN')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to do something if $user is not an instance of User: adding something like where 1=2 to the query or (better) mention that the security component must be used to require a user to be authenticated to access this route.


## Custom Extension

Adding an extension is as easy as registering a service in your `app/config/services.yml` file and create the class you need.
Copy link
Contributor

@teohhanhui teohhanhui Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this whole paragraph can be removed.


Custom extension must implement the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface`
and / or the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryItemExtensionInterface`
interfaces, depending if you are asking for a collection of items or just an item.
Copy link
Contributor

@teohhanhui teohhanhui Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

, to be run when querying for a collection of items and when querying for an item respectively.

API Platform Core provides a system to extend queries on items and collections.

Extensions are specific to Doctrine, and therefore, the Doctrine ORM support must be enabled to use this feature.
If you use custom providers it's up to you to implement your own extension system or not.
Copy link
Contributor

@teohhanhui teohhanhui Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid this kind of formatting in Markdown. If it's supposed to be the same paragraph, then there should not be a line break, and should continue immediately after use this feature. on the previous line.

If you want a new paragraph, an extra line break is necessary (1 blank line in between).

If instead you want it to simply start on the next line, it's necessary to have 2 trailing spaces on the previous line. (But I don't think we want this.)


Adding an extension is as easy as registering a service in your `app/config/services.yml` file and create the class you need.

Custom extension must implement the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryCollectionExtensionInterface`
Copy link
Contributor

@teohhanhui teohhanhui Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom extensions

## Example

In the following example, we will see how to always get the offers owned by the current user. We will set up an exception, whenever the user has the `ROLE_ADMIN`.
Given these two entities:
Copy link
Contributor

@teohhanhui teohhanhui Oct 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same concern about formatting here... (and many other places below)


Notice the priority level for the `api_platform.doctrine.orm.query_extension.collection` tag. When an extension implements the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryResultCollectionExtensionInterface` or the `ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryResultItemExtensionInterface` interface to return results by itself, any lower priority extension will not be executed. Because the pagination is enabled by default with a priority of 8, the priority of the `app.doctrine.orm.query_extension.current_user` service must be at least 9 to ensure its execution.

### Note
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest: ### Blocking Anonymous Users


### Note

This example adds a WHERE condition only when a fully authenticated user without ROLE_ADMIN tries to access to a resource. That mean it return without restriction any item or collection for every other user. You need to ensure that your user is authenticated to access the two endpoints.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

 a `WHERE` clause [...] without the `ROLE_ADMIN` role [...]. It means that anonymous users will be able to access to all data. To prevent this potential security issue, the API must ensure that the current user is authenticated.


This example adds a WHERE condition only when a fully authenticated user without ROLE_ADMIN tries to access to a resource. That mean it return without restriction any item or collection for every other user. You need to ensure that your user is authenticated to access the two endpoints.

A way of doing it, is with the access control :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To secure the access to endpoints, use the following access control rule:

(No space before the colon)

security:
# ...
firewalls:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can remove this whole block (the # ... before is enough).

Copy link
Member

dunglas commented Oct 20, 2016

@GregoireHebert can you rebase please?

Copy link
Contributor

@toofff toofff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After your rebase.

Change Previous chapter in file core/serialization-groups-and-relations.md by Previous chapter: [Extensions](extensions.md)

- { path: ^/users, roles: IS_AUTHENTICATED_FULLY }
```

Previous chapter: [Filters](filters.md) Next chapter: [Serialization Groups and Relations](serialization-groups-and-relations.md) No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add break line between Previous chapter and Next chapter, please.

index.md Outdated
7. [Serialization Groups and Relations](core/serialization-groups-and-relations.md)
7. [Extensions](core/extensions.md)
1. [Custom Extension](core/extensions.md#custom-extension)
2. [Filter upon the current user](core/extensions.md#filter-upon-the-current-user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is a tltle Filter upon the current user in the file core/extensions.md?


If you use [custom data providers](data-providers.md), they must support extensions and be aware of active extensions to work properly.

## Example
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should really be a level 2 title?

Copy link
Contributor Author

@GregoireHebert GregoireHebert Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What level would you use ?

Copy link
Contributor

@Simperfit Simperfit Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exemple

blabla

Exemple

blabla maybe ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i prefer ### Example @Simperfit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if the title in the file /index.md is Filter upon the current user then it is necessary to put ## Filter upon the current user instead of ## Example.

And change anchor in the file /index.md.

Copy link
Contributor Author

@GregoireHebert GregoireHebert Oct 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index.md has a "Filter upon the current user" chapter, because it is what it is, it suits better than "Example" when that what a user wants to do, he finds it instantly. that's why the "Filter upon the current user" is linked with the example anchor (By the way, it used to be a "filter upon the current user" anchor, but @dunglas made me change it for "Example". For the anchor level, since it appear on the summary, I thought it had to be the same level than the previous anchor.

toofff reacted with thumbs up emoji
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor Author

seems good to me !

Copy link
Contributor

@Simperfit Simperfit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks about that @GregoireHebert


If you use [custom data providers](data-providers.md), they must support extensions and be aware of active extensions to work properly.

## Example
Copy link
Contributor

@Simperfit Simperfit Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exemple

blabla

Exemple

blabla maybe ?

use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;

final class CurrentUserExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface
Copy link
Contributor

@Simperfit Simperfit Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@api-platform/core-team IIUC the docs should contain best practices, is it one to implements both in one class ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok to me for this use case. Creating two classes with duplicated code isn't ok however. So I would keep it as is.

index.md Outdated
1. [Creating Custom Doctrine ORM Filters](core/filters.md#creating-custom-doctrine-orm-filters)
2. [Overriding Extraction of Properties from the Request](core/filters.md#overriding-extraction-of-properties-from-the-request)
6. [Serialization Groups and Relations](core/serialization-groups-and-relations.md)
6. [Extensions](core/extensions.md)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this section after Data Providers? It's an advanced topic, it should not be too early in the book.

Copy link
Contributor Author

GregoireHebert commented Oct 24, 2016 •
edited
Loading

TOC changed :)

@dunglas dunglas merged commit cbe664d into api-platform:master Oct 24, 2016
Copy link
Member

dunglas commented Oct 24, 2016

Thank you @GregoireHebert!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dunglas dunglas dunglas approved these changes

+3 more reviewers

@toofff toofff toofff requested changes

@Simperfit Simperfit Simperfit left review comments

@teohhanhui teohhanhui teohhanhui requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /