Add documentation about doctrine query extensions #120

GregoireHebert · 2016-10-07T12:42:43Z

No description provided.

soyuka · 2016-10-07T13:31:03Z

see #107

dunglas · 2016-10-07T14:07:03Z

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

soyuka · 2016-10-07T14:11:49Z

+1 should've written related

dunglas · 2016-10-07T14:09:19Z

core/extensions.md

@@ -0,0 +1,164 @@
+# Extensions
+
+API Platform Core provides a system to extends queries on items and collections readings.


extends [...] ~~readings~~

dunglas · 2016-10-07T14:09:39Z

core/extensions.md

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


a custom [...]

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

dunglas · 2016-10-07T14:11:25Z

core/extensions.md

+
+## 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.


adding an extension
create the class?

dunglas · 2016-10-07T14:12:07Z

core/extensions.md

+
+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`


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

dunglas · 2016-10-07T14:14:20Z

core/extensions.md

+
+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.


depending if you are asking for

dunglas · 2016-10-07T14:21:41Z

core/extensions.md

+    }
+
+    /**
+     * Add where condition


Adds the WHERE condition.

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

dunglas · 2016-10-07T14:23:42Z

core/extensions.md

+        }
+
+        $classMetadata = $queryBuilder->getEntityManager()->getClassMetadata($resourceClass);
+        $user = $this->token->getToken()->getUser();


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.

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

dunglas · 2016-10-07T14:27:32Z

core/extensions.md

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


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.

dunglas · 2016-10-07T14:29:22Z

core/extensions.md

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


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

dunglas · 2016-10-07T14:31:41Z

core/extensions.md

+            - { 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.


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.

dunglas · 2016-10-07T14:40:17Z

core/extensions.md

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


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

dunglas · 2016-10-07T14:41:30Z

core/extensions.md

+        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());


It's still a security issue and a bad practice. You have to use parameters: http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/query-builder.html#binding-parameters-to-your-query

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

GregoireHebert · 2016-10-07T15:15:14Z

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

dunglas · 2016-10-12T18:07:03Z

core/extensions.md

+            - { 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.


`api_platform.doctrine.orm.query_extension.collection `

dunglas · 2016-10-12T18:07:18Z

core/extensions.md

+```
+
+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.


`api_platform.doctrine.orm.query_extension.item`

dunglas · 2016-10-12T18:08:30Z

core/extensions.md

+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.


api_platform.doctrine.orm.query_extension.collection tag

dunglas · 2016-10-12T18:08:56Z

core/extensions.md

+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,


"There is a case" can be removed

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

dunglas · 2016-10-12T18:11:23Z

core/extensions.md

+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.


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.

dunglas · 2016-10-12T18:14:49Z

core/extensions.md

+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.


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

dunglas · 2016-10-12T18:16:18Z

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.
+
+## Filter upon the current user


I would change by Example

dunglas · 2016-10-12T18:16:34Z

core/extensions.md

+{
+    // ...
+}
+


Extra blank line to remove

dunglas · 2016-10-12T18:16:47Z

core/extensions.md

+
+    //...
+}
+


extra blank line to remove

dunglas · 2016-10-12T18:19:01Z

core/extensions.md

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


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.

teohhanhui · 2016-10-14T07:05:00Z

core/extensions.md

+
+## 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.


I think this whole paragraph can be removed.

teohhanhui · 2016-10-14T07:12:32Z

core/extensions.md

+
+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.


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

teohhanhui · 2016-10-14T07:15:57Z

core/extensions.md

+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.


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.)

teohhanhui · 2016-10-14T07:16:27Z

core/extensions.md

+
+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`


Custom extensions

teohhanhui · 2016-10-14T07:17:26Z

core/extensions.md

+## 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:


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

dunglas · 2016-10-17T12:33:57Z

core/extensions.md

+
+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


I suggest: ### Blocking Anonymous Users

dunglas · 2016-10-17T12:35:29Z

core/extensions.md

+
+### 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.


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.

dunglas · 2016-10-17T12:36:10Z

core/extensions.md

+
+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 :


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

(No space before the colon)

dunglas · 2016-10-17T12:36:36Z

core/extensions.md

+security:
+    # ...
+
+    firewalls:


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

dunglas · 2016-10-20T16:45:42Z

@GregoireHebert can you rebase please?

toofff

After your rebase.

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

toofff · 2016-10-20T16:59:50Z

core/extensions.md

+        - { path: ^/users, roles: IS_AUTHENTICATED_FULLY }
+```
+
+Previous chapter: [Filters](filters.md) Next chapter: [Serialization Groups and Relations](serialization-groups-and-relations.md)


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

toofff · 2016-10-20T17:07:34Z

index.md

-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)


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

toofff · 2016-10-20T17:10:20Z

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


It should really be a level 2 title?

What level would you use ?

Exemple

blabla

Exemple

blabla maybe ?

Yes, i prefer ### Example @Simperfit

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.

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.

GregoireHebert · 2016-10-20T18:11:55Z

seems good to me !

Simperfit

Thanks about that @GregoireHebert

Simperfit · 2016-10-20T18:21:37Z

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


Exemple

blabla

Exemple

blabla maybe ?

Simperfit · 2016-10-20T18:23:51Z

core/extensions.md

+use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
+use Symfony\Component\Security\Core\Authorization\AuthorizationChecker;
+
+final class CurrentUserExtension implements QueryCollectionExtensionInterface, QueryItemExtensionInterface


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

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.

dunglas · 2016-10-24T12:31:09Z

index.md

    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)


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

GregoireHebert · 2016-10-24T12:49:36Z

TOC changed :)

dunglas · 2016-10-24T13:20:50Z

Thank you @GregoireHebert!

dunglas requested changes Oct 7, 2016

View reviewed changes

dunglas requested changes Oct 12, 2016

View reviewed changes

teohhanhui suggested changes Oct 14, 2016

View reviewed changes

dunglas requested changes Oct 17, 2016

View reviewed changes

GregoireHebert force-pushed the master branch from 4fe0268 to f2efd9e Compare October 20, 2016 08:43

dunglas approved these changes Oct 20, 2016

View reviewed changes

toofff suggested changes Oct 20, 2016

View reviewed changes

GregoireHebert force-pushed the master branch from f2efd9e to e15c723 Compare October 20, 2016 18:05

Simperfit reviewed Oct 20, 2016

View reviewed changes

GregoireHebert added 9 commits October 21, 2016 08:22

Add documentation about doctrine query extensions

92ddec1

Update extensions.md

ffef6c8

Update extensions.md

e768e62

Update extensions.md

a9b640e

adds explanations about priorities on extensions.md

46e0a5c

Rephrasing, cleaning and adding security

03b7153

style: improve .md style

4583492

Update extensions.md

f016600

style: new break line on extension chapter and fix index

2d805d7

GregoireHebert force-pushed the master branch from e15c723 to 2d805d7 Compare October 21, 2016 06:23

dunglas reviewed Oct 24, 2016

View reviewed changes

GregoireHebert added 3 commits October 24, 2016 14:40

style: move extension.md TOC further

7775488

style: move extension.md TOC further

c325bbc

style: move extension.md TOC further

7e25652

dunglas merged commit cbe664d into api-platform:master Oct 24, 2016

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

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


		## 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.


		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`


		## 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.


		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`


		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


		### 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.


		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 :


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

		## Example

Uh oh!

Add documentation about doctrine query extensions #120

Add documentation about doctrine query extensions #120

Uh oh!

Conversation

GregoireHebert commented Oct 7, 2016

Uh oh!

soyuka commented Oct 7, 2016

Uh oh!

dunglas commented Oct 7, 2016

Uh oh!

soyuka commented Oct 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas Oct 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GregoireHebert commented Oct 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dunglas Oct 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

dunglas Oct 7, 2016 •

edited

Loading

dunglas Oct 7, 2016 •

edited

Loading

dunglas Oct 12, 2016 •

edited

Loading

dunglas Oct 12, 2016 •

edited

Loading