Skip to content

Commit 72dde50

Browse files
dunglasdunglas
authored
Merge pull request #738 from dunglas/quality
Refactor XML loaders and various quality fixes
2 parents 399d4b3 + f7ecfa1 commit 72dde50

21 files changed

+177
-426
lines changed

‎composer.json‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
{
22
"name": "api-platform/core",
33
"type": "library",
4-
"description": "JSON-LD / Hydra REST API for Symfony",
5-
"keywords": ["REST", "API", "JSON", "JSON-LD", "Hydra"],
4+
"description": "The ultimate solution to create web APIs.",
5+
"keywords": ["REST", "API", "JSON", "JSON-LD", "Hydra", "Swagger", "HAL"],
66
"homepage": "https://api-platform.com",
77
"license": "MIT",
88
"authors": [
@@ -39,6 +39,7 @@
3939
"phpdocumentor/reflection-docblock": "^3.0",
4040
"psr/log": "^1.0",
4141
"symfony/cache": "^3.1",
42+
"symfony/config": "^2.7",
4243
"symfony/dependency-injection": "^2.7 || ^3.0",
4344
"symfony/doctrine-bridge": "^2.8 || ^3.0",
4445
"symfony/phpunit-bridge": "^2.7 || ^3.0",
@@ -49,12 +50,12 @@
4950
"symfony/twig-bundle": "^2.8 || ^3.1"
5051
},
5152
"suggest": {
52-
"symfony/twig-bundle": "To have a human-readable documentation relying on Swagger UI.",
5353
"friendsofsymfony/user-bundle": "To use the FOSUserBundle bridge.",
54-
"nelmio/api-doc-bundle": "To have the api sandbox & documentation.",
5554
"phpdocumentor/reflection-docblock": "To support extracting metadata from PHPDoc.",
5655
"psr/cache-implementation": "To use metadata caching.",
57-
"symfony/cache": "To have metadata caching when using Symfony integration."
56+
"symfony/cache": "To have metadata caching when using Symfony integration.",
57+
"symfony/config": "To load XML configuration files.",
58+
"symfony/twig-bundle": "To use the Swagger UI integration."
5859
},
5960
"autoload": {
6061
"psr-4": { "ApiPlatform\\Core\\": "src/" }

‎src/Bridge/Doctrine/Orm/Util/QueryChecker.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,12 @@
2020
* @author Teoh Han Hui <[email protected]>
2121
* @author Vincent Chalamon <[email protected]>
2222
*/
23-
abstract class QueryChecker
23+
final class QueryChecker
2424
{
25+
private function __construct()
26+
{
27+
}
28+
2529
/**
2630
* Determines whether the query builder uses a HAVING clause.
2731
*

‎src/Bridge/Doctrine/Orm/Util/QueryJoinParser.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,12 @@
2323
* @author Teoh Han Hui <[email protected]>
2424
* @author Vincent Chalamon <[email protected]>
2525
*/
26-
abstract class QueryJoinParser
26+
final class QueryJoinParser
2727
{
28+
private function __construct()
29+
{
30+
}
31+
2832
/**
2933
* Gets the class metadata from a given join alias.
3034
*

‎src/Metadata/Resource/Factory/XmlResourceMetadataFactory.php‎

Lines changed: 47 additions & 181 deletions
Original file line numberDiff line numberDiff line change
@@ -14,27 +14,27 @@
1414
use ApiPlatform\Core\Exception\InvalidArgumentException;
1515
use ApiPlatform\Core\Exception\ResourceClassNotFoundException;
1616
use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
17+
use Symfony\Component\Config\Util\XmlUtils;
1718

1819
/**
19-
* Creates a resource metadata from xml {@see Resource} configuration.
20+
* Creates a resource metadata from XML {@see Resource} configuration.
2021
*
2122
* @author Antoine Bluchet <[email protected]>
23+
* @author Kévin Dunglas <[email protected]>
2224
*/
2325
final class XmlResourceMetadataFactory implements ResourceMetadataFactoryInterface
2426
{
25-
private $xmlParser;
27+
const RESOURCE_SCHEMA = __DIR__.'/../../schema/metadata.xsd';
28+
2629
private $paths;
2730
private $decorated;
2831

29-
const RESOURCE_SCHEMA = __DIR__.'/../../../schema/metadata.xsd';
30-
3132
/**
3233
* @param string[] $paths
3334
* @param ResourceMetadataFactoryInterface|null $decorated
3435
*/
3536
public function __construct(array $paths, ResourceMetadataFactoryInterface $decorated = null)
3637
{
37-
$this->xmlParser = new \DOMDocument();
3838
$this->paths = $paths;
3939
$this->decorated = $decorated;
4040
}
@@ -53,170 +53,63 @@ public function create(string $resourceClass) : ResourceMetadata
5353
}
5454
}
5555

56-
try {
57-
new \ReflectionClass($resourceClass);
58-
} catch (\ReflectionException $reflectionException) {
59-
return $this->handleNotFound($parentResourceMetadata, $resourceClass);
60-
}
61-
62-
$metadata = null;
63-
64-
foreach ($this->paths as $path) {
65-
$resources = $this->getResourcesDom($path);
66-
67-
$internalErrors = libxml_use_internal_errors(true);
68-
69-
if (false === @$resources->schemaValidate(self::RESOURCE_SCHEMA)) {
70-
throw new InvalidArgumentException(sprintf('XML Schema loaded from path %s is not valid! Errors: %s', realpath($path), implode("\n", $this->getXmlErrors($internalErrors))));
71-
}
72-
73-
libxml_clear_errors();
74-
libxml_use_internal_errors($internalErrors);
75-
76-
foreach ($resources->getElementsByTagName('resource') as $resource) {
77-
$class = $resource->getAttribute('class');
78-
79-
if ($resourceClass !== $class) {
80-
continue;
81-
}
82-
83-
$metadata = $resource;
84-
85-
break 2;
86-
}
87-
}
88-
89-
if (null === $metadata) {
56+
if (!class_exists($resourceClass) || empty($metadata = $this->getMetadata($resourceClass))) {
9057
return $this->handleNotFound($parentResourceMetadata, $resourceClass);
9158
}
9259

93-
$xpath = new \DOMXpath($resources);
94-
95-
$metadata = [
96-
'shortName' => $metadata->getAttribute('shortName') ?: null,
97-
'description' => $metadata->getAttribute('description') ?: null,
98-
'iri' => $metadata->getAttribute('iri') ?: null,
99-
'itemOperations' => $this->getOperations($xpath->query('./itemOperations/operation', $metadata)) ?: null,
100-
'collectionOperations' => $this->getOperations($xpath->query('./collectionOperations/operation', $metadata)) ?: null,
101-
'attributes' => $this->getAttributes($xpath->query('./attributes/attribute', $metadata)),
102-
];
103-
104-
if (!$parentResourceMetadata) {
105-
return new ResourceMetadata(
106-
$metadata['shortName'],
107-
$metadata['description'],
108-
$metadata['iri'],
109-
$metadata['itemOperations'],
110-
$metadata['collectionOperations'],
111-
$metadata['attributes']
112-
);
113-
}
114-
115-
$resourceMetadata = $parentResourceMetadata;
116-
117-
foreach (['shortName', 'description', 'itemOperations', 'collectionOperations', 'iri', 'attributes'] as $property) {
118-
if (!isset($metadata[$property])) {
119-
continue;
120-
}
121-
122-
$resourceMetadata = $this->createWith($resourceMetadata, $property, $metadata[$property]);
123-
}
124-
125-
return $resourceMetadata;
60+
return null === $parentResourceMetadata ? new ResourceMetadata(...$metadata) : $this->update($parentResourceMetadata, $metadata);
12661
}
12762

12863
/**
129-
* Creates a DOMDocument based on `resource` tags of a file-loaded xml document.
64+
* Extracts metadata from the XML tree.
13065
*
131-
* @param string $path the xml file path
66+
* @param string $resourceClass
13267
*
133-
* @return \DOMDocument
68+
* @return array
13469
*/
135-
private function getResourcesDom(string $path) : \DOMDocument
70+
private function getMetadata(string $resourceClass) : array
13671
{
137-
$doc = new \DOMDocument('1.0', 'utf-8');
138-
$root = $doc->createElement('resources');
139-
$doc->appendChild($root);
140-
141-
$this->xmlParser->loadXML(file_get_contents($path));
142-
143-
$xpath = new \DOMXpath($this->xmlParser);
144-
$resources = $xpath->query('//resource');
145-
146-
foreach ($resources as $resource) {
147-
$root->appendChild($doc->importNode($resource, true));
148-
}
149-
150-
return $doc;
151-
}
152-
153-
/**
154-
* Get operations from xml.
155-
*
156-
* @param \DOMNodeList $query
157-
*
158-
* @return array|null
159-
*/
160-
private function getOperations(\DOMNodeList $query)
161-
{
162-
$operations = [];
163-
foreach ($query as $operation) {
164-
$key = $operation->getAttribute('key');
165-
$operations[$key] = [
166-
'method' => $operation->getAttribute('method'),
167-
];
72+
foreach ($this->paths as $path) {
73+
try {
74+
$domDocument = XmlUtils::loadFile($path, self::RESOURCE_SCHEMA);
75+
} catch (\InvalidArgumentException $e) {
76+
throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
77+
}
16878

169-
$path = $operation->getAttribute('path');
79+
$xml = simplexml_import_dom($domDocument);
80+
foreach ($xml->resource as $resource) {
81+
if ($resourceClass !== (string) $resource['class']) {
82+
continue;
83+
}
17084

171-
if ($path) {
172-
$operations[$key]['path'] = $path;
85+
return [
86+
(string) $resource['shortName'] ?? null,
87+
(string) $resource['description'] ?? null,
88+
(string) $resource['iri'] ?? null,
89+
$this->getAttributes($resource, 'itemOperation') ?: null,
90+
$this->getAttributes($resource, 'collectionOperation') ?: null,
91+
$this->getAttributes($resource, 'attribute') ?: null,
92+
];
17393
}
17494
}
17595

176-
return $operations ?: null;
96+
return [];
17797
}
17898

17999
/**
180-
* Get Attributes.
100+
* Recursively transforms an attribute structure into an associative array.
181101
*
182-
* @param \DOMNodeList $query
102+
* @param \SimpleXMLElement $resource
103+
* @param string $elementName
183104
*
184-
* @return array|null
105+
* @return array
185106
*/
186-
private function getAttributes(\DOMNodeList $query)
107+
private function getAttributes(\SimpleXMLElement $resource, string $elementName) : array
187108
{
188109
$attributes = [];
189-
foreach ($query as $attribute) {
190-
$key = $attribute->getAttribute('key');
191-
$attributes[$key] = $this->recursiveAttributes($attribute, $attributes[$key]);
192-
}
193-
194-
return $attributes ?: null;
195-
}
196-
197-
/**
198-
* Transforms random attributes in an array
199-
* <element (key="key"|int)>\DOMNodeList|\DOMText</element>.
200-
*
201-
* @param \DOMElement $element
202-
* @param array
203-
*
204-
* @return array|string
205-
*/
206-
private function recursiveAttributes(\DOMElement $element, &$attributes)
207-
{
208-
foreach ($element->childNodes as $child) {
209-
if ($child instanceof \DOMText) {
210-
if ($child->isWhitespaceInElementContent()) {
211-
continue;
212-
}
213-
214-
$attributes = $child->nodeValue;
215-
break;
216-
}
217-
218-
$key = $child->getAttribute('key') ?: count($attributes);
219-
$attributes[$key] = $child->childNodes->length ? $this->recursiveAttributes($child, $attributes[$key]) : $child->value;
110+
foreach ($resource->$elementName as $attribute) {
111+
$value = isset($attribute->attribute[0]) ? $this->getAttributes($attribute, 'attribute') : (string) $attribute;
112+
isset($attribute['name']) ? $attributes[(string) $attribute['name']] = $value : $attributes[] = $value;
220113
}
221114

222115
return $attributes;
@@ -245,47 +138,20 @@ private function handleNotFound(ResourceMetadata $parentPropertyMetadata = null,
245138
* Creates a new instance of metadata if the property is not already set.
246139
*
247140
* @param ResourceMetadata $resourceMetadata
248-
* @param string $property
249-
* @param mixed $value
141+
* @param array $metadata
250142
*
251143
* @return ResourceMetadata
252144
*/
253-
private function createWith(ResourceMetadata $resourceMetadata, string $property, $value) : ResourceMetadata
145+
private function update(ResourceMetadata $resourceMetadata, array $metadata) : ResourceMetadata
254146
{
255-
$getter = 'get'.ucfirst($property);
256-
257-
if (null !== $resourceMetadata->$getter()) {
258-
return $resourceMetadata;
259-
}
260-
261-
$wither = 'with'.ucfirst($property);
262-
263-
return $resourceMetadata->$wither($value);
264-
}
147+
foreach (['shortName', 'description', 'iri', 'itemOperations', 'collectionOperations', 'attributes'] as $key => $property) {
148+
if (null === $metadata[$key] || null !== $resourceMetadata->{'get'.ucfirst($property)}()) {
149+
continue;
150+
}
265151

266-
/**
267-
* Returns the XML errors of the internal XML parser.
268-
*
269-
* @param bool $internalErrors
270-
*
271-
* @return array An array of errors
272-
*/
273-
private function getXmlErrors($internalErrors)
274-
{
275-
$errors = [];
276-
foreach (libxml_get_errors() as $error) {
277-
$errors[] = sprintf('[%s %s] %s (in %s - line %d, column %d)',
278-
LIBXML_ERR_WARNING == $error->level ? 'WARNING' : 'ERROR',
279-
$error->code,
280-
trim($error->message),
281-
$error->file ?: 'n/a',
282-
$error->line,
283-
$error->column
284-
);
152+
$resourceMetadata = $resourceMetadata->{'with'.ucfirst($property)}($metadata[$key]);
285153
}
286-
libxml_clear_errors();
287-
libxml_use_internal_errors($internalErrors);
288154

289-
return $errors;
155+
return $resourceMetadata;
290156
}
291157
}

0 commit comments

Comments
 (0)