Add data providers and data persisters matches to the debug panel by antograssiot · Pull Request #2262 · api-platform/core

antograssiot · 2018-10-20T20:04:17Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets
License	MIT
Doc PR

Even if I do like the data providers system, I generally find hard to debug them and end in dumping in vendor chain data providers.

This PR aims to provide a more developer friendly way to debug them by improving the Api Platform panel in Symfony debug toolbar

Up to now I've implemented item/collection/subresources data provider and data persister
If you like the idea, I'll add tests before merging this.

antograssiot · 2018-10-20T20:12:12Z

Add tabs to ease navigation and here is a screenshot, easier to see than the gif

dunglas · 2018-10-21T11:10:04Z

Very good idea!

dunglas · 2018-10-21T11:11:18Z

src/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersister.php

+/**
+ * @author Anthony GRASSIOT <antograssiot@free.fr>
+ */
+class TraceableChainDataPersister implements DataPersisterInterface


Suggested change

class TraceableChainDataPersister implements DataPersisterInterface

final class TraceableChainDataPersister implements DataPersisterInterface

dunglas · 2018-10-21T11:12:33Z

src/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersister.php

+    {
+        if ($dataPersister instanceof ChainDataPersister) {
+            $this->decorated = $dataPersister;
+            $reflection = new \ReflectionProperty(ChainDataPersister::class, 'persisters');


Maybe can you make this parameter public and marked @internal?

src/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersister.php

dunglas · 2018-10-21T11:15:46Z

src/Bridge/Symfony/Bundle/DataProvider/TraceableChainCollectionDataProvider.php

+    public function __construct(CollectionDataProviderInterface $collectionDataProvider)
+    {
+        if ($collectionDataProvider instanceof ChainCollectionDataProvider) {
+            $reflection = new \ReflectionProperty(ChainCollectionDataProvider::class, 'dataProviders');


src/Bridge/Symfony/Bundle/DataProvider/TraceableChainItemDataProvider.php

dunglas · 2018-10-21T11:21:37Z

src/Bridge/Symfony/Bundle/DataProvider/TraceableChainSubresourceDataProvider.php

+    public function __construct(SubresourceDataProviderInterface $subresourceDataProvider)
+    {
+        if ($subresourceDataProvider instanceof ChainSubresourceDataProvider) {
+            $reflection = new \ReflectionProperty(ChainSubresourceDataProvider::class, 'dataProviders');


src/Bridge/Symfony/Bundle/DataProvider/TraceableChainSubresourceDataProvider.php

dunglas · 2018-10-21T11:22:35Z

src/Bridge/Symfony/Bundle/Resources/config/data_provider_debug.xml

@@ -0,0 +1,21 @@
+<?xml version="1.0" ?>


can me merged with the previous file isn't it?

I wanted to activate those listener only if the api platform profiler is enable and debug is on but I can simplify and remove extra files if you prefer, let me know.

got it finaly 👍

got it finally 👍. I updated the file and added test on the extension. just waiting for github hangover to be finished

antograssiot · 2018-10-21T12:12:29Z

I've updated following your comments @dunglas, ~~except the "extra" loops and the separates files waiting for more comments after my answers~~. thanks for your review

antograssiot · 2018-10-22T18:09:02Z

everything has been updated here, just waiting for github webhooks to be back on the scene I guess now

src/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersister.php

src/Bridge/Symfony/Bundle/DataProvider/TraceableChainCollectionDataProvider.php

src/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersister.php

antograssiot · 2018-10-25T06:17:27Z

branched rebased.
A generic way to trace Extension would be great too I think, maybe something like a WrappedExtension class that would decorate every extension so we could check if they where call (in custom data providers too). Then all wrapped extension would be collected and displayed in a new tab. WDYT ?

dunglas · 2018-11-01T21:40:23Z

Thank you very much @antograssiot, very nice improvement

* ApiPlatformExtension cleanup * Remove a now useless composer hack * remove unset attributes key in normalization context * Add tests on doctrine collection purge * Fix 2285 - force Yaml::dump to dump empty array as actual empty array * improve graphiql CSS * Prefix root resource route_prefix to sub-resources Unit test also added * Bump PHPStan analysis to level 6 (api-platform#2272) * Bump PHPStan analysis to level 6 * Bump PHPStan analysis to level 6 * Bump PHPStan analysis to level 6 * Deprecate dead code in QueryJoinParser and remove internal usage * Add a non regression test for api-platform#2285 api-platform#2286 * Ability to modify response headers (mainly cache related headers) (api-platform#2288) * Allow user-defined cache headers A user may wish to define response cache headers in a custom controller or per resource. E.g. a /entity/random endpoint should have a max-age of 0 * Override cache max-age and shared-max-age Add annotation attributes for cache_headers which allows max_age and shared_max_age to be assigned per resource. * Remove unused property The property was added when I thought it may be good to have the option outside of 'attributes' - decided to put the option in attributes instead and failed to remove this property. * Code style fixes * Test code style fix * Review changes applied - Order of constructor arguments + default null value - Remove 'throws' annotation - setting $resourceCacheHeaders with default fallback to empty array - Improved conditional statements * Add cacheHeaders Attribute annotation * Accidental typo * Alphabetical order Updated attribute annotation to alphabetical order - moved the property into alphabetical order as well * Add data providers and data persisters matches to the debug panel (api-platform#2262) * Add data providers and data persisters matches to the debug panel * Add tests on traceable providers and persisters * Use internal public properties * Merge config file and add tests on Extension * Avoid extra loop in data persister/providers * remove duplicated code * Fix after rebasing * Allow an input and an output for a given resource class * [Bugfix] Remove type Error

antograssiot force-pushed the data-collectors branch 4 times, most recently from 71f5437 to 5bbeb47 Compare October 21, 2018 10:38

dunglas requested changes Oct 21, 2018

View reviewed changes

antograssiot force-pushed the data-collectors branch from 5bbeb47 to a59ce7e Compare October 21, 2018 11:42

antograssiot force-pushed the data-collectors branch 4 times, most recently from b4871f2 to 3000eda Compare October 22, 2018 15:08

dunglas requested changes Oct 22, 2018

View reviewed changes

src/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersister.php Show resolved Hide resolved

src/Bridge/Symfony/Bundle/DataProvider/TraceableChainCollectionDataProvider.php Show resolved Hide resolved

antograssiot commented Oct 23, 2018

View reviewed changes

src/Bridge/Symfony/Bundle/DataPersister/TraceableChainDataPersister.php Show resolved Hide resolved

dunglas approved these changes Oct 23, 2018

View reviewed changes

antograssiot force-pushed the data-collectors branch from f362c97 to d717d34 Compare October 25, 2018 05:51

antograssiot added 7 commits October 31, 2018 20:18

Add data providers and data persisters matches to the debug panel

e927ccf

Add tests on traceable providers and persisters

0cedc52

Use internal public properties

bc17c73

Merge config file and add tests on Extension

bd9115b

Avoid extra loop in data persister/providers

72cc4c6

remove duplicated code

687233d

Fix after rebasing

35493dd

antograssiot force-pushed the data-collectors branch from d717d34 to 35493dd Compare October 31, 2018 19:18

dunglas merged commit d5c9d0b into api-platform:master Nov 1, 2018

antograssiot deleted the data-collectors branch November 2, 2018 13:21

	class TraceableChainDataPersister implements DataPersisterInterface
	final class TraceableChainDataPersister implements DataPersisterInterface

Uh oh!

Comments

Conversation

antograssiot commented Oct 20, 2018

Uh oh!

antograssiot commented Oct 20, 2018

Uh oh!

dunglas commented Oct 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

antograssiot commented Oct 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

antograssiot commented Oct 22, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

antograssiot commented Oct 25, 2018

Uh oh!

dunglas commented Nov 1, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

antograssiot commented Oct 21, 2018 •

edited

Loading