From 994f81fd8696a95104b5c1e3817a82173b029b4c Mon Sep 17 00:00:00 2001 From: Christophe Coevoet Date: Fri, 27 Jun 2014 01:18:27 +0200 Subject: [PATCH] Refactored the CssSelector to remove the circular object graph This allows the translator and its extensions to be garbage collected based on the refcount rather than requiring the garbage collector run, making it much more likely to happen at the end of the CssSelector::toXPath call. --- .../XPath/Extension/ExtensionInterface.php | 2 + .../XPath/Extension/NodeExtension.php | 63 ++++++++++--------- .../CssSelector/XPath/Translator.php | 4 +- 3 files changed, 36 insertions(+), 33 deletions(-) diff --git a/src/Symfony/Component/CssSelector/XPath/Extension/ExtensionInterface.php b/src/Symfony/Component/CssSelector/XPath/Extension/ExtensionInterface.php index 65ab287770f2..22312659429a 100644 --- a/src/Symfony/Component/CssSelector/XPath/Extension/ExtensionInterface.php +++ b/src/Symfony/Component/CssSelector/XPath/Extension/ExtensionInterface.php @@ -24,6 +24,8 @@ interface ExtensionInterface /** * Returns node translators. * + * These callables will receive the node as first argument and the translator as second argument. + * * @return callable[] */ public function getNodeTranslators(); diff --git a/src/Symfony/Component/CssSelector/XPath/Extension/NodeExtension.php b/src/Symfony/Component/CssSelector/XPath/Extension/NodeExtension.php index f86f2b967266..d71baaa96bcd 100644 --- a/src/Symfony/Component/CssSelector/XPath/Extension/NodeExtension.php +++ b/src/Symfony/Component/CssSelector/XPath/Extension/NodeExtension.php @@ -29,11 +29,6 @@ class NodeExtension extends AbstractExtension const ATTRIBUTE_NAME_IN_LOWER_CASE = 2; const ATTRIBUTE_VALUE_IN_LOWER_CASE = 4; - /** - * @var Translator - */ - private $translator; - /** * @var int */ @@ -42,12 +37,10 @@ class NodeExtension extends AbstractExtension /** * Constructor. * - * @param Translator $translator - * @param int $flags + * @param int $flags */ - public function __construct(Translator $translator, $flags = 0) + public function __construct($flags = 0) { - $this->translator = $translator; $this->flags = $flags; } @@ -100,33 +93,36 @@ public function getNodeTranslators() /** * @param Node\SelectorNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translateSelector(Node\SelectorNode $node) + public function translateSelector(Node\SelectorNode $node, Translator $translator) { - return $this->translator->nodeToXPath($node->getTree()); + return $translator->nodeToXPath($node->getTree()); } /** * @param Node\CombinedSelectorNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translateCombinedSelector(Node\CombinedSelectorNode $node) + public function translateCombinedSelector(Node\CombinedSelectorNode $node, Translator $translator) { - return $this->translator->addCombination($node->getCombinator(), $node->getSelector(), $node->getSubSelector()); + return $translator->addCombination($node->getCombinator(), $node->getSelector(), $node->getSubSelector()); } /** * @param Node\NegationNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translateNegation(Node\NegationNode $node) + public function translateNegation(Node\NegationNode $node, Translator $translator) { - $xpath = $this->translator->nodeToXPath($node->getSelector()); - $subXpath = $this->translator->nodeToXPath($node->getSubSelector()); + $xpath = $translator->nodeToXPath($node->getSelector()); + $subXpath = $translator->nodeToXPath($node->getSubSelector()); $subXpath->addNameTest(); if ($subXpath->getCondition()) { @@ -138,34 +134,37 @@ public function translateNegation(Node\NegationNode $node) /** * @param Node\FunctionNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translateFunction(Node\FunctionNode $node) + public function translateFunction(Node\FunctionNode $node, Translator $translator) { - $xpath = $this->translator->nodeToXPath($node->getSelector()); + $xpath = $translator->nodeToXPath($node->getSelector()); - return $this->translator->addFunction($xpath, $node); + return $translator->addFunction($xpath, $node); } /** * @param Node\PseudoNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translatePseudo(Node\PseudoNode $node) + public function translatePseudo(Node\PseudoNode $node, Translator $translator) { - $xpath = $this->translator->nodeToXPath($node->getSelector()); + $xpath = $translator->nodeToXPath($node->getSelector()); - return $this->translator->addPseudoClass($xpath, $node->getIdentifier()); + return $translator->addPseudoClass($xpath, $node->getIdentifier()); } /** * @param Node\AttributeNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translateAttribute(Node\AttributeNode $node) + public function translateAttribute(Node\AttributeNode $node, Translator $translator) { $name = $node->getAttribute(); $safe = $this->isSafeName($name); @@ -181,37 +180,39 @@ public function translateAttribute(Node\AttributeNode $node) $attribute = $safe ? '@'.$name : sprintf('attribute::*[name() = %s]', Translator::getXpathLiteral($name)); $value = $node->getValue(); - $xpath = $this->translator->nodeToXPath($node->getSelector()); + $xpath = $translator->nodeToXPath($node->getSelector()); if ($this->hasFlag(self::ATTRIBUTE_VALUE_IN_LOWER_CASE)) { $value = strtolower($value); } - return $this->translator->addAttributeMatching($xpath, $node->getOperator(), $attribute, $value); + return $translator->addAttributeMatching($xpath, $node->getOperator(), $attribute, $value); } /** * @param Node\ClassNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translateClass(Node\ClassNode $node) + public function translateClass(Node\ClassNode $node, Translator $translator) { - $xpath = $this->translator->nodeToXPath($node->getSelector()); + $xpath = $translator->nodeToXPath($node->getSelector()); - return $this->translator->addAttributeMatching($xpath, '~=', '@class', $node->getName()); + return $translator->addAttributeMatching($xpath, '~=', '@class', $node->getName()); } /** * @param Node\HashNode $node + * @param Translator $translator * * @return XPathExpr */ - public function translateHash(Node\HashNode $node) + public function translateHash(Node\HashNode $node, Translator $translator) { - $xpath = $this->translator->nodeToXPath($node->getSelector()); + $xpath = $translator->nodeToXPath($node->getSelector()); - return $this->translator->addAttributeMatching($xpath, '=', '@id', $node->getId()); + return $translator->addAttributeMatching($xpath, '=', '@id', $node->getId()); } /** diff --git a/src/Symfony/Component/CssSelector/XPath/Translator.php b/src/Symfony/Component/CssSelector/XPath/Translator.php index 5a8eb99017a1..4676677ea488 100644 --- a/src/Symfony/Component/CssSelector/XPath/Translator.php +++ b/src/Symfony/Component/CssSelector/XPath/Translator.php @@ -76,7 +76,7 @@ public function __construct(ParserInterface $parser = null) $this->mainParser = $parser ?: new Parser(); $this - ->registerExtension(new Extension\NodeExtension($this)) + ->registerExtension(new Extension\NodeExtension()) ->registerExtension(new Extension\CombinationExtension()) ->registerExtension(new Extension\FunctionExtension()) ->registerExtension(new Extension\PseudoClassExtension()) @@ -207,7 +207,7 @@ public function nodeToXPath(NodeInterface $node) throw new ExpressionErrorException(sprintf('Node "%s" not supported.', $node->getNodeName())); } - return call_user_func($this->nodeTranslators[$node->getNodeName()], $node); + return call_user_func($this->nodeTranslators[$node->getNodeName()], $node, $this); } /**