Skip to content

Commit a46a768

Browse files
soyukasoyuka
authored
fix(symfony): serialization of closure inside the profiler (api-platform#7054)
1 parent b0fa229 commit a46a768

File tree

4 files changed

+102
-134
lines changed

4 files changed

+102
-134
lines changed

‎src/Symfony/Bundle/DataCollector/RequestDataCollector.php

Lines changed: 20 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515

1616
use ApiPlatform\Metadata\ApiResource;
1717
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
18-
use ApiPlatform\State\Util\RequestAttributesExtractor;
19-
use Composer\InstalledVersions;
20-
use PackageVersions\Versions;
2118
use Psr\Container\ContainerInterface;
2219
use Symfony\Component\HttpFoundation\Request;
2320
use Symfony\Component\HttpFoundation\Response;
@@ -42,17 +39,22 @@ public function collect(Request $request, Response $response, ?\Throwable $excep
4239
if ($request->attributes->get('_graphql', false)) {
4340
$resourceClasses = array_keys($request->attributes->get('_graphql_args', []));
4441
} else {
45-
$resourceClasses = array_filter([$request->attributes->get('_api_resource_class')]);
42+
$cl = $request->attributes->get('_api_resource_class');
43+
$resourceClasses = $cl ? [$cl] : [];
4644
}
4745

48-
$requestAttributes = RequestAttributesExtractor::extractAttributes($request);
49-
if (isset($requestAttributes['previous_data'])) {
50-
$requestAttributes['previous_data'] = $this->cloneVar($requestAttributes['previous_data']);
46+
$this->data['operation_name'] = $request->attributes->get('_api_operation_name');
47+
$this->data['acceptable_content_types'] = $request->getAcceptableContentTypes();
48+
$this->data['resources'] = array_map(fn (string $resourceClass): DataCollected => $this->collectDataByResource($resourceClass), $resourceClasses);
49+
50+
$parameters = [];
51+
if ($operation = $request->attributes->get('_api_operation')) {
52+
foreach ($operation->getParameters() ?? [] as $key => $param) {
53+
$parameters[$key] = $this->cloneVar($param);
54+
}
5155
}
5256

53-
$this->data['request_attributes'] = $requestAttributes;
54-
$this->data['acceptable_content_types'] = $request->getAcceptableContentTypes();
55-
$this->data['resources'] = array_map(fn (string $resourceClass): DataCollected => $this->collectDataByResource($resourceClass, $request), $resourceClasses);
57+
$this->data['parameters'] = $parameters;
5658
}
5759

5860
private function setFilters(ApiResource $resourceMetadata, int $index, array &$filters, array &$counters): void
@@ -68,40 +70,6 @@ private function setFilters(ApiResource $resourceMetadata, int $index, array &$f
6870
}
6971
}
7072

71-
// TODO: 4.1 remove Versions as its deprecated
72-
public function getVersion(): ?string
73-
{
74-
if (class_exists(InstalledVersions::class)) {
75-
return InstalledVersions::getPrettyVersion('api-platform/symfony') ?? InstalledVersions::getPrettyVersion('api-platform/core');
76-
}
77-
78-
if (!class_exists(Versions::class)) {
79-
return null;
80-
}
81-
82-
try {
83-
$version = strtok(Versions::getVersion('api-platform/symfony'), '@');
84-
} catch (\OutOfBoundsException) {
85-
$version = false;
86-
}
87-
88-
if (false === $version) {
89-
try {
90-
$version = strtok(Versions::getVersion('api-platform/core'), '@');
91-
} catch (\OutOfBoundsException) {
92-
$version = false;
93-
}
94-
}
95-
96-
if (false === $version) {
97-
return null;
98-
}
99-
100-
preg_match('/^v(.*?)$/', $version, $output);
101-
102-
return $output[1] ?? $version;
103-
}
104-
10573
/**
10674
* {@inheritdoc}
10775
*/
@@ -120,9 +88,14 @@ public function getAcceptableContentTypes(): array
12088
return $this->data['acceptable_content_types'] ?? [];
12189
}
12290

123-
public function getRequestAttributes(): array
91+
public function getOperationName(): ?string
92+
{
93+
return $this->data['operation_name'] ?? null;
94+
}
95+
96+
public function getParameters(): array
12497
{
125-
return $this->data['request_attributes'] ?? [];
98+
return $this->data['parameters'] ?? [];
12699
}
127100

128101
public function getResources(): array
@@ -138,7 +111,7 @@ public function reset(): void
138111
$this->data = [];
139112
}
140113

141-
private function collectDataByResource(string $resourceClass, Request $request): DataCollected
114+
private function collectDataByResource(string $resourceClass): DataCollected
142115
{
143116
$resourceMetadataCollection = $resourceClass ? $this->metadataFactory->create($resourceClass) : [];
144117
$filters = [];

‎src/Symfony/Bundle/Resources/views/DataCollector/request.html.twig

Lines changed: 75 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,33 @@
11
{% extends '@WebProfiler/Profiler/layout.html.twig' %}
22

3-
{% macro operationLine(key, operation, actualOperationName) %}
3+
{% macro operationLine(key, operation, actualOperationName, n) %}
4+
{% set context_id = 'operation-' ~ n %}
45
<tr>
5-
<th scope="row"{% if key == actualOperationName %} class="status-success"{% endif %}>{{ key }}</th>
6-
<td{% if key == actualOperationName %} class="status-success"{% endif %}>{{- profiler_dump(operation, 1) -}}</td>
6+
<th style="width: 40%" scope="row"{% if key == actualOperationName %} class="status-success"{% endif %}>{{ key }}</th>
7+
<td style="width: 60%" {% if key == actualOperationName %} class="status-success"{% endif %}>
8+
<a class="btn btn-link text-small sf-toggle" data-toggle-selector="#{{ context_id }}" data-toggle-alt-content="Hide context">Show context</a>
9+
<div id="{{ context_id }}" class="context sf-toggle-content sf-toggle-hidden">
10+
{{- profiler_dump(operation, 2) -}}
11+
</div>
12+
</td>
713
</tr>
814
{% endmacro %}
915

10-
{% macro operationTable(object, name, actualOperationName) %}
16+
{% macro operationTable(object, actualOperationName) %}
1117
{% import _self as apiPlatform %}
1218
<table>
1319
<thead>
1420
<tr>
15-
<th scope="col" class="key">{% if name is defined %}{{ name|capitalize }}{% endif %}Operations</th>
16-
<th scope="col">Attributes</th>
21+
<th scope="col" class="key">{% if name is defined %}{{ name|capitalize }}{% endif %}Name</th>
22+
<th scope="col">Operation</th>
1723
</tr>
1824
</thead>
1925

2026
<tbody>
27+
{% set n = 0 %}
2128
{% for key, itemOperation in object %}
22-
{{ apiPlatform.operationLine(key, itemOperation, actualOperationName) }}
29+
{% set n = n + 1 %}
30+
{{ apiPlatform.operationLine(key, itemOperation, actualOperationName, n) }}
2331
{% else %}
2432
<tr>
2533
<td colspan="2" class="text-muted">
@@ -83,12 +91,6 @@
8391
{% endset %}
8492

8593
{% set text %}
86-
{% if collector.version %}
87-
<div class="sf-toolbar-info-piece">
88-
<b>Version</b>
89-
<span>{{ collector.version }}</span>
90-
</div>
91-
{% endif %}
9294
{% if collector.resources|length == 0 %}
9395
<div class="sf-toolbar-info-piece">
9496
<b>Resource Class</b>
@@ -139,10 +141,11 @@
139141
</div>
140142
</div>
141143
{% if dataCollected.resourceMetadataCollection is not empty %}
142-
<h3>Resources</h3>
144+
<h2>Resources</h2>
143145
<div class="tab-content metadata-tab-content">
144146
<div class="sf-tabs">
145-
{% for resourceMetadata in dataCollected.resourceMetadataCollection %}
147+
{% for index, resourceMetadata in dataCollected.resourceMetadataCollection %}
148+
{% set context_id = 'context-' ~ resourceMetadata.resource.shortName ~ index %}
146149
<div class="tab">
147150
<h3 class="tab-title">
148151
{{ resourceMetadata.resource.uriTemplate ?: resourceMetadata.resource.shortName }}
@@ -151,16 +154,69 @@
151154
<table>
152155
<thead>
153156
<tr>
154-
<th scope="col" class="key">Resource</th>
157+
<th scope="col" class="key">Name</th>
158+
<th>ApiResource</th>
155159
</tr>
156160
</thead>
157161
<tbody>
158162
<tr>
159-
<td>{{- profiler_dump(resourceMetadata.resource, 1) -}}</td>
163+
<td>{{ resourceMetadata.resource.shortName }}</td>
164+
<td>
165+
<div>
166+
<a class="btn btn-link text-small sf-toggle" data-toggle-selector="#{{ context_id }}" data-toggle-alt-content="Hide context">Show context</a>
167+
<div id="{{ context_id }}" class="context sf-toggle-content sf-toggle-hidden">
168+
{{- profiler_dump(resourceMetadata.resource, 2) -}}
169+
</div>
170+
</div>
171+
</td>
160172
</tr>
161173
</tbody>
162174
</table>
163-
{{ apiPlatform.operationTable(resourceMetadata.operations, '', dataCollected.requestAttributes.operation_name|default('')) }}
175+
<h2>Operations</h2>
176+
{{ apiPlatform.operationTable(resourceMetadata.operations, collector.operationName) }}
177+
<h2>Parameters</h2>
178+
<table>
179+
<thead>
180+
<tr>
181+
<th scope="col" class="key">Key</th>
182+
<th scope="col">Value</th>
183+
<th scope="col">Parameter</th>
184+
</tr>
185+
</thead>
186+
<tbody>
187+
{% if collector.parameters %}
188+
{% for key, parameter in collector.parameters %}
189+
{% set context_id = 'parameter-' ~ key %}
190+
<tr>
191+
<td>
192+
{{ key }}
193+
</td>
194+
<td>
195+
{% if parameter.extraProperties['_api_values'] is defined %}
196+
{{ dump(parameter.extraProperties['_api_values']) }}
197+
{% else %}
198+
∅
199+
{% endif %}
200+
</td>
201+
<td>
202+
<a class="btn btn-link text-small sf-toggle" data-toggle-selector="#{{ context_id }}" data-toggle-alt-content="Hide context">Show context</a>
203+
<div id="{{ context_id }}" class="context sf-toggle-content sf-toggle-hidden">
204+
{{- profiler_dump(parameter, 3) -}}
205+
</div>
206+
207+
</td>
208+
</tr>
209+
{% endfor %}
210+
{% else %}
211+
<tr>
212+
<td class="text-muted" colspan="2">
213+
No available parameters declared for this resource.
214+
</td>
215+
</tr>
216+
{% endif %}
217+
</tbody>
218+
</table>
219+
<h2>Filters</h2>
164220
<table>
165221
<thead>
166222
<tr>
@@ -178,7 +234,7 @@
178234
{% if ignored_filter %}
179235
<span class="newline text-muted">ignored filter</span>
180236
{% else %}
181-
{{ dump(filter) }}
237+
{{ filter }}
182238
{% endif %}
183239
</td>
184240
</tr>
@@ -200,4 +256,3 @@
200256
{% endif %}
201257
{% endfor %}
202258
{% endblock %}
203-

‎tests/Symfony/Bundle/DataCollector/RequestDataCollectorTest.php

Lines changed: 3 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
use ApiPlatform\Metadata\Resource\ResourceMetadataCollection;
2121
use ApiPlatform\Symfony\Bundle\DataCollector\RequestDataCollector;
2222
use ApiPlatform\Tests\Fixtures\DummyEntity;
23-
use Composer\InstalledVersions;
24-
use PackageVersions\Versions;
2523
use PHPUnit\Framework\MockObject\MockObject;
2624
use PHPUnit\Framework\TestCase;
2725
use Prophecy\PhpUnit\ProphecyTrait;
@@ -73,7 +71,6 @@ public function testNoResourceClass(): void
7371
$this->response
7472
);
7573

76-
$this->assertEquals([], $dataCollector->getRequestAttributes());
7774
$this->assertEquals(['foo', 'bar'], $dataCollector->getAcceptableContentTypes());
7875
$this->assertEquals([], $dataCollector->getResources());
7976
}
@@ -88,14 +85,13 @@ public function testNotCallingCollect(): void
8885
$this->filterLocator->reveal()
8986
);
9087

91-
$this->assertEquals([], $dataCollector->getRequestAttributes());
9288
$this->assertEquals([], $dataCollector->getAcceptableContentTypes());
9389
$this->assertEquals([], $dataCollector->getResources());
9490
}
9591

9692
public function testWithResource(): void
9793
{
98-
$this->apiResourceClassWillReturn(DummyEntity::class, ['_api_operation_name' => 'get', '_api_receive' => true]);
94+
$this->apiResourceClassWillReturn(DummyEntity::class, ['_api_operation_name' => 'get']);
9995
$this->request->attributes = $this->attributes->reveal();
10096

10197
$this->filterLocator->has('foo')->willReturn(false)->shouldBeCalled();
@@ -112,14 +108,6 @@ public function testWithResource(): void
112108
$this->response
113109
);
114110

115-
$this->assertEquals([
116-
'resource_class' => DummyEntity::class,
117-
'has_composite_identifier' => false,
118-
'operation_name' => 'get',
119-
'receive' => true,
120-
'respond' => true,
121-
'persist' => true,
122-
], $dataCollector->getRequestAttributes());
123111
$this->assertEquals(['foo', 'bar'], $dataCollector->getAcceptableContentTypes());
124112

125113
$resource = $dataCollector->getResources()[0];
@@ -147,59 +135,11 @@ public function testWithResourceWithTraceables(): void
147135
);
148136
}
149137

150-
public function testVersionCollection(): void
151-
{
152-
$this->apiResourceClassWillReturn(DummyEntity::class);
153-
154-
$this->filterLocator->has('a_filter')->willReturn(false);
155-
$this->filterLocator->has('foo')->willReturn(false);
156-
157-
$dataCollector = new RequestDataCollector(
158-
$this->metadataFactory->reveal(),
159-
$this->filterLocator->reveal(),
160-
);
161-
162-
$dataCollector->collect(
163-
$this->request->reveal(),
164-
$this->response
165-
);
166-
167-
if (class_exists(InstalledVersions::class)) {
168-
$this->assertTrue(null !== $dataCollector->getVersion());
169-
} else {
170-
$this->assertSame(null !== $dataCollector->getVersion(), class_exists(Versions::class));
171-
}
172-
}
173-
174-
public function testWithPreviousData(): void
175-
{
176-
$data = new \stdClass();
177-
$data->a = $data;
178-
179-
$this->apiResourceClassWillReturn(DummyEntity::class, ['_api_operation_name' => 'get', '_api_receive' => true, 'previous_data' => $data]);
180-
$this->request->attributes = $this->attributes->reveal();
181-
182-
$this->filterLocator->has('foo')->willReturn(false);
183-
$this->filterLocator->has('a_filter')->willReturn(true);
184-
$this->filterLocator->get('a_filter')->willReturn(new \stdClass());
185-
186-
$dataCollector = new RequestDataCollector(
187-
$this->metadataFactory->reveal(),
188-
$this->filterLocator->reveal()
189-
);
190-
191-
$dataCollector->collect(
192-
$this->request->reveal(),
193-
$this->response
194-
);
195-
196-
$this->assertArrayHasKey('previous_data', $requestAttributes = $dataCollector->getRequestAttributes());
197-
$this->assertNotSame($requestAttributes['previous_data']->data, $requestAttributes['previous_data']);
198-
}
199-
200138
private function apiResourceClassWillReturn(?string $data, array $context = []): void
201139
{
202140
$this->attributes->get('_api_resource_class')->shouldBeCalled()->willReturn($data);
141+
$this->attributes->get('_api_operation_name')->shouldBeCalled()->willReturn($context['_api_operation_name'] ?? null);
142+
$this->attributes->get('_api_operation')->shouldBeCalled()->willReturn($context['_api_operation'] ?? new Get());
203143
$this->attributes->get('_graphql', false)->shouldBeCalled()->willReturn(false);
204144
$this->attributes->all()->willReturn([
205145
'_api_resource_class' => $data,

‎tests/Symfony/Bundle/Twig/ApiPlatformProfilerPanelTest.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,9 @@ public function testProfilerGeneralLayout(): void
124124
$tabContent = $crawler->filter('.tab:nth-of-type(1)');
125125
$this->assertStringEndsWith('Dummy', trim($tabContent->filter('h3')->html()), 'the resource shortname should be displayed.');
126126

127-
$this->assertCount(3, $tabContent->filter('table'));
128-
$this->assertSame('Resource', $tabContent->filter('table:first-of-type thead th:first-of-type')->html());
129-
$this->assertSame('Operations', $tabContent->filter('table:nth-of-type(2) thead th:first-of-type')->html());
130-
$this->assertSame('Filters', $tabContent->filter('table:nth-of-type(3) thead th:first-of-type')->html());
127+
$this->assertCount(4, $tabContent->filter('table'));
128+
$this->assertSame('Name', $tabContent->filter('table:first-of-type thead th:first-of-type')->html());
129+
$this->assertSame('Name', $tabContent->filter('table:nth-of-type(2) thead th:first-of-type')->html());
130+
$this->assertSame('Key', $tabContent->filter('table:nth-of-type(3) thead th:first-of-type')->html());
131131
}
132132
}

0 commit comments

Comments
 (0)