Refactor XML loaders and various quality fixes

composer.json

@@ -1,8 +1,8 @@
 {
     "name": "api-platform/core",
     "type": "library",
-    "description": "JSON-LD / Hydra REST API for Symfony",
-    "keywords": ["REST", "API", "JSON", "JSON-LD", "Hydra"],
+    "description": "The ultimate solution to create web APIs.",
+    "keywords": ["REST", "API", "JSON", "JSON-LD", "Hydra", "Swagger", "HAL"],
     "homepage": "https://api-platform.com",
     "license": "MIT",
     "authors": [
@@ -39,6 +39,7 @@
         "phpdocumentor/reflection-docblock": "^3.0",
         "psr/log": "^1.0",
         "symfony/cache": "^3.1",
+        "symfony/config": "^2.7",
         "symfony/dependency-injection": "^2.7 || ^3.0",
         "symfony/doctrine-bridge": "^2.8 || ^3.0",
         "symfony/phpunit-bridge": "^2.7 || ^3.0",
@@ -49,12 +50,12 @@
         "symfony/twig-bundle": "^2.8 || ^3.1"
     },
     "suggest": {
-        "symfony/twig-bundle": "To have a human-readable documentation relying on Swagger UI.",
         "friendsofsymfony/user-bundle": "To use the FOSUserBundle bridge.",
-        "nelmio/api-doc-bundle": "To have the api sandbox & documentation.",
         "phpdocumentor/reflection-docblock": "To support extracting metadata from PHPDoc.",
         "psr/cache-implementation": "To use metadata caching.",
-        "symfony/cache": "To have metadata caching when using Symfony integration."
+        "symfony/cache": "To have metadata caching when using Symfony integration.",
+        "symfony/config": "To load XML configuration files.",
+        "symfony/twig-bundle": "To use the Swagger UI integration."
     },
     "autoload": {
         "psr-4": { "ApiPlatform\\Core\\": "src/" }

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

@@ -20,8 +20,12 @@
  * @author Teoh Han Hui <teohhanhui@gmail.com>
  * @author Vincent Chalamon <vincentchalamon@gmail.com>
  */
-abstract class QueryChecker
+final class QueryChecker
 {
+    private function __construct()
+    {
+    }
+
     /**
      * Determines whether the query builder uses a HAVING clause.
      *

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

@@ -23,8 +23,12 @@
  * @author Teoh Han Hui <teohhanhui@gmail.com>
  * @author Vincent Chalamon <vincentchalamon@gmail.com>
  */
-abstract class QueryJoinParser
+final class QueryJoinParser
 {
+    private function __construct()
+    {
+    }
+
     /**
      * Gets the class metadata from a given join alias.
      *

src/Metadata/Resource/Factory/XmlResourceMetadataFactory.php

@@ -14,27 +14,27 @@
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\Exception\ResourceClassNotFoundException;
 use ApiPlatform\Core\Metadata\Resource\ResourceMetadata;
+use Symfony\Component\Config\Util\XmlUtils;
 
 /**
- * Creates a resource metadata from xml {@see Resource} configuration.
+ * Creates a resource metadata from XML {@see Resource} configuration.
  *
  * @author Antoine Bluchet <soyuka@gmail.com>
+ * @author Kévin Dunglas <dunglas@gmail.com>
  */
 final class XmlResourceMetadataFactory implements ResourceMetadataFactoryInterface
 {
-    private $xmlParser;
+    const RESOURCE_SCHEMA = __DIR__.'/../../schema/metadata.xsd';
+
     private $paths;
     private $decorated;
 
-    const RESOURCE_SCHEMA = __DIR__.'/../../../schema/metadata.xsd';
-
     /**
      * @param string[]                              $paths
      * @param ResourceMetadataFactoryInterface|null $decorated
      */
     public function __construct(array $paths, ResourceMetadataFactoryInterface $decorated = null)
     {
-        $this->xmlParser = new \DOMDocument();
         $this->paths = $paths;
         $this->decorated = $decorated;
     }
@@ -53,170 +53,63 @@ public function create(string $resourceClass) : ResourceMetadata
             }
         }
 
-        try {
-            new \ReflectionClass($resourceClass);
-        } catch (\ReflectionException $reflectionException) {
-            return $this->handleNotFound($parentResourceMetadata, $resourceClass);
-        }
-
-        $metadata = null;
-
-        foreach ($this->paths as $path) {
-            $resources = $this->getResourcesDom($path);
-
-            $internalErrors = libxml_use_internal_errors(true);
-
-            if (false === @$resources->schemaValidate(self::RESOURCE_SCHEMA)) {
-                throw new InvalidArgumentException(sprintf('XML Schema loaded from path %s is not valid! Errors: %s', realpath($path), implode("\n", $this->getXmlErrors($internalErrors))));
-            }
-
-            libxml_clear_errors();
-            libxml_use_internal_errors($internalErrors);
-
-            foreach ($resources->getElementsByTagName('resource') as $resource) {
-                $class = $resource->getAttribute('class');
-
-                if ($resourceClass !== $class) {
-                    continue;
-                }
-
-                $metadata = $resource;
-
-                break 2;
-            }
-        }
-
-        if (null === $metadata) {
+        if (!class_exists($resourceClass) || empty($metadata = $this->getMetadata($resourceClass))) {
             return $this->handleNotFound($parentResourceMetadata, $resourceClass);
         }
 
-        $xpath = new \DOMXpath($resources);
-
-        $metadata = [
-          'shortName' => $metadata->getAttribute('shortName') ?: null,
-          'description' => $metadata->getAttribute('description') ?: null,
-          'iri' => $metadata->getAttribute('iri') ?: null,
-          'itemOperations' => $this->getOperations($xpath->query('./itemOperations/operation', $metadata)) ?: null,
-          'collectionOperations' => $this->getOperations($xpath->query('./collectionOperations/operation', $metadata)) ?: null,
-          'attributes' => $this->getAttributes($xpath->query('./attributes/attribute', $metadata)),
-        ];
-
-        if (!$parentResourceMetadata) {
-            return new ResourceMetadata(
-              $metadata['shortName'],
-              $metadata['description'],
-              $metadata['iri'],
-              $metadata['itemOperations'],
-              $metadata['collectionOperations'],
-              $metadata['attributes']
-            );
-        }
-
-        $resourceMetadata = $parentResourceMetadata;
-
-        foreach (['shortName', 'description', 'itemOperations', 'collectionOperations', 'iri', 'attributes'] as $property) {
-            if (!isset($metadata[$property])) {
-                continue;
-            }
-
-            $resourceMetadata = $this->createWith($resourceMetadata, $property, $metadata[$property]);
-        }
-
-        return $resourceMetadata;
+        return null === $parentResourceMetadata ? new ResourceMetadata(...$metadata) : $this->update($parentResourceMetadata, $metadata);
     }
 
     /**
-     * Creates a DOMDocument based on `resource` tags of a file-loaded xml document.
+     * Extracts metadata from the XML tree.
      *
-     * @param string $path the xml file path
+     * @param string $resourceClass
      *
-     * @return \DOMDocument
+     * @return array
      */
-    private function getResourcesDom(string $path) : \DOMDocument
+    private function getMetadata(string $resourceClass) : array
     {
-        $doc = new \DOMDocument('1.0', 'utf-8');
-        $root = $doc->createElement('resources');
-        $doc->appendChild($root);
-
-        $this->xmlParser->loadXML(file_get_contents($path));
-
-        $xpath = new \DOMXpath($this->xmlParser);
-        $resources = $xpath->query('//resource');
-
-        foreach ($resources as $resource) {
-            $root->appendChild($doc->importNode($resource, true));
-        }
-
-        return $doc;
-    }
-
-    /**
-     * Get operations from xml.
-     *
-     * @param \DOMNodeList $query
-     *
-     * @return array|null
-     */
-    private function getOperations(\DOMNodeList $query)
-    {
-        $operations = [];
-        foreach ($query as $operation) {
-            $key = $operation->getAttribute('key');
-            $operations[$key] = [
-          'method' => $operation->getAttribute('method'),
-        ];
+        foreach ($this->paths as $path) {
+            try {
+                $domDocument = XmlUtils::loadFile($path, self::RESOURCE_SCHEMA);
+            } catch (\InvalidArgumentException $e) {
+                throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
+            }
 
-            $path = $operation->getAttribute('path');
+            $xml = simplexml_import_dom($domDocument);
+            foreach ($xml->resource as $resource) {
+                if ($resourceClass !== (string) $resource['class']) {
+                    continue;
+                }
 
-            if ($path) {
-                $operations[$key]['path'] = $path;
+                return [
+                    (string) $resource['shortName'] ?? null,
+                    (string) $resource['description'] ?? null,
+                    (string) $resource['iri'] ?? null,
+                    $this->getAttributes($resource, 'itemOperation') ?: null,
+                    $this->getAttributes($resource, 'collectionOperation') ?: null,
+                    $this->getAttributes($resource, 'attribute') ?: null,
+                ];
             }
         }
 
-        return $operations ?: null;
+        return [];
     }
 
     /**
-     * Get Attributes.
+     * Recursively transforms an attribute structure into an associative array.
      *
-     * @param \DOMNodeList $query
+     * @param \SimpleXMLElement $resource
+     * @param string            $elementName
      *
-     * @return array|null
+     * @return array
      */
-    private function getAttributes(\DOMNodeList $query)
+    private function getAttributes(\SimpleXMLElement $resource, string $elementName) : array
     {
         $attributes = [];
-        foreach ($query as $attribute) {
-            $key = $attribute->getAttribute('key');
-            $attributes[$key] = $this->recursiveAttributes($attribute, $attributes[$key]);
-        }
-
-        return $attributes ?: null;
-    }
-
-    /**
-     * Transforms random attributes in an array
-     * <element (key="key"|int)>\DOMNodeList|\DOMText</element>.
-     *
-     * @param \DOMElement $element
-     * @param array
-     *
-     * @return array|string
-     */
-    private function recursiveAttributes(\DOMElement $element, &$attributes)
-    {
-        foreach ($element->childNodes as $child) {
-            if ($child instanceof \DOMText) {
-                if ($child->isWhitespaceInElementContent()) {
-                    continue;
-                }
-
-                $attributes = $child->nodeValue;
-                break;
-            }
-
-            $key = $child->getAttribute('key') ?: count($attributes);
-            $attributes[$key] = $child->childNodes->length ? $this->recursiveAttributes($child, $attributes[$key]) : $child->value;
+        foreach ($resource->$elementName as $attribute) {
+            $value = isset($attribute->attribute[0]) ? $this->getAttributes($attribute, 'attribute') : (string) $attribute;
+            isset($attribute['name']) ? $attributes[(string) $attribute['name']] = $value : $attributes[] = $value;
         }
 
         return $attributes;
@@ -245,47 +138,20 @@ private function handleNotFound(ResourceMetadata $parentPropertyMetadata = null,
      * Creates a new instance of metadata if the property is not already set.
      *
      * @param ResourceMetadata $resourceMetadata
-     * @param string           $property
-     * @param mixed            $value
+     * @param array            $metadata
      *
      * @return ResourceMetadata
      */
-    private function createWith(ResourceMetadata $resourceMetadata, string $property, $value) : ResourceMetadata
+    private function update(ResourceMetadata $resourceMetadata, array $metadata) : ResourceMetadata
     {
-        $getter = 'get'.ucfirst($property);
-
-        if (null !== $resourceMetadata->$getter()) {
-            return $resourceMetadata;
-        }
-
-        $wither = 'with'.ucfirst($property);
-
-        return $resourceMetadata->$wither($value);
-    }
+        foreach (['shortName', 'description', 'iri', 'itemOperations', 'collectionOperations', 'attributes'] as $key => $property) {
+            if (null === $metadata[$key] || null !== $resourceMetadata->{'get'.ucfirst($property)}()) {
+                continue;
+            }
 
-    /**
-     * Returns the XML errors of the internal XML parser.
-     *
-     * @param bool $internalErrors
-     *
-     * @return array An array of errors
-     */
-    private function getXmlErrors($internalErrors)
-    {
-        $errors = [];
-        foreach (libxml_get_errors() as $error) {
-            $errors[] = sprintf('[%s %s] %s (in %s - line %d, column %d)',
-                LIBXML_ERR_WARNING == $error->level ? 'WARNING' : 'ERROR',
-                $error->code,
-                trim($error->message),
-                $error->file ?: 'n/a',
-                $error->line,
-                $error->column
-            );
+            $resourceMetadata = $resourceMetadata->{'with'.ucfirst($property)}($metadata[$key]);
         }
-        libxml_clear_errors();
-        libxml_use_internal_errors($internalErrors);
 
-        return $errors;
+        return $resourceMetadata;
     }
 }