From 242bada43de9f3fb6c7cb0c999e497e881762bb4 Mon Sep 17 00:00:00 2001 From: Adam Robak Date: Thu, 7 May 2015 14:37:13 +0200 Subject: [PATCH] external parser approach changed, tests fixed, refactoring --- PortableInfobox.setup.php | 1 + services/Parser/Nodes/Node.php | 40 +++++++++++------------- services/Parser/Nodes/NodeComparison.php | 12 +++---- services/Parser/Nodes/NodeData.php | 8 ++--- services/Parser/Nodes/NodeFooter.php | 6 ++-- services/Parser/Nodes/NodeGroup.php | 11 +++---- services/Parser/Nodes/NodeHeader.php | 6 ++-- services/Parser/Nodes/NodeImage.php | 19 +++++------ services/Parser/Nodes/NodeSet.php | 8 ++--- services/Parser/SimpleParser.php | 13 ++++++++ services/Parser/XmlParser.php | 10 +++--- tests/XmlParserTest.php | 23 +++++++------- 12 files changed, 79 insertions(+), 78 deletions(-) create mode 100644 services/Parser/SimpleParser.php diff --git a/PortableInfobox.setup.php b/PortableInfobox.setup.php index 4592c12..4a711f7 100644 --- a/PortableInfobox.setup.php +++ b/PortableInfobox.setup.php @@ -19,6 +19,7 @@ $wgAutoloadClasses[ 'PortableInfoboxRenderService' ] = $dir . 'services/Portable // parser $wgAutoloadClasses[ 'Wikia\\PortableInfobox\\Parser\\ExternalParser'] = $dir . 'services/Parser/ExternalParser.php'; +$wgAutoloadClasses[ 'Wikia\\PortableInfobox\\Parser\\SimpleParser'] = $dir . 'services/Parser/SimpleParser.php'; $wgAutoloadClasses[ 'Wikia\\PortableInfobox\\Parser\\XmlParser'] = $dir . 'services/Parser/XmlParser.php'; $wgAutoloadClasses[ 'Wikia\\PortableInfobox\\Parser\\DummyParser'] = $dir . 'services/Parser/DummyParser.php'; $wgAutoloadClasses[ 'Wikia\\PortableInfobox\\Parser\\MediaWikiParserService'] = $dir . 'services/Parser/MediaWikiParserService.php'; diff --git a/services/Parser/Nodes/Node.php b/services/Parser/Nodes/Node.php index 8d2e372..57ae0de 100644 --- a/services/Parser/Nodes/Node.php +++ b/services/Parser/Nodes/Node.php @@ -2,6 +2,7 @@ namespace Wikia\PortableInfobox\Parser\Nodes; use Wikia\PortableInfobox\Parser\ExternalParser; +use Wikia\PortableInfobox\Parser\SimpleParser; class Node { @@ -20,6 +21,16 @@ class Node { $this->infoboxData = $infoboxData; } + /** + * @return ExternalParser + */ + public function getExternalParser() { + if ( !isset( $this->externalParser ) ) { + $this->setExternalParser( new SimpleParser() ); + } + return $this->externalParser; + } + /** * @param mixed $externalParser */ @@ -32,7 +43,7 @@ class Node { } public function getData() { - return [ 'value' => (string) $this->xmlNode ]; + return [ 'value' => (string)$this->xmlNode ]; } public function isEmpty( $data ) { @@ -47,36 +58,21 @@ class Node { } if ( !$value ) { if ( $xmlNode->{self::DEFAULT_TAG_NAME} ) { - $value = (string) $xmlNode->{self::DEFAULT_TAG_NAME}; - $value = $this->parseWithExternalParser( $value, true ); + $value = (string)$xmlNode->{self::DEFAULT_TAG_NAME}; + $value = $this->getExternalParser()->parseRecursive( $value ); } } return $value; } - protected function getXmlAttribute( \SimpleXMLElement $xmlNode, $attribute ) { - if( isset( $xmlNode[ $attribute ] ) ) - return (string) $xmlNode[ $attribute ]; + protected function getXmlAttribute( \SimpleXMLElement $xmlNode, $attribute ) { + if ( isset( $xmlNode[ $attribute ] ) ) + return (string)$xmlNode[ $attribute ]; return null; } - /** - * @FIXME: regardless of what is the final approach, this code needs to be explained - * WHY it does the things it does. Here. In docblock. Or by phrasing it explicitly with - * class and method names. - */ - protected function parseWithExternalParser( $data, $recursive = true ) { - if ( !empty( $data ) && !empty( $this->externalParser ) ) { - if ( $recursive ) { - return $this->externalParser->parseRecursive( $data ); - } - return $this->externalParser->parse( $data ); - } - return $data; - } - protected function getInfoboxData( $key ) { $data = isset( $this->infoboxData[ $key ] ) ? $this->infoboxData[ $key ] : null; - return $this->parseWithExternalParser( $data ); + return $this->getExternalParser()->parse( $data ); } } diff --git a/services/Parser/Nodes/NodeComparison.php b/services/Parser/Nodes/NodeComparison.php index aa244bd..05fc46c 100644 --- a/services/Parser/Nodes/NodeComparison.php +++ b/services/Parser/Nodes/NodeComparison.php @@ -3,26 +3,22 @@ namespace Wikia\PortableInfobox\Parser\Nodes; use Wikia\PortableInfobox\Parser\XmlParser; -class NodeComparison extends Node { +class NodeComparison extends Node { public function getData() { $nodeFactory = new XmlParser( $this->infoboxData ); if ( $this->externalParser ) { $nodeFactory->setExternalParser( $this->externalParser ); } - $data = []; - $data['value'] = $nodeFactory->getDataFromNodes( $this->xmlNode ); - return $data; + return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ]; } public function isEmpty( $data ) { - foreach ( $data['value'] as $group ) { - if ( $group['isEmpty'] == false ) { + foreach ( $data[ 'value' ] as $group ) { + if ( $group[ 'isEmpty' ] == false ) { return false; } } return true; } - - } diff --git a/services/Parser/Nodes/NodeData.php b/services/Parser/Nodes/NodeData.php index 00fd4eb..0a10bca 100644 --- a/services/Parser/Nodes/NodeData.php +++ b/services/Parser/Nodes/NodeData.php @@ -4,9 +4,9 @@ namespace Wikia\PortableInfobox\Parser\Nodes; class NodeData extends Node { public function getData() { - $data = []; - $data['label'] = $this->parseWithExternalParser( (string) $this->xmlNode->{self::LABEL_TAG_NAME}, true ); - $data['value'] = $this->getValueWithDefault( $this->xmlNode ); - return $data; + return [ + 'label' => $this->getExternalParser()->parseRecursive( (string) $this->xmlNode->{self::LABEL_TAG_NAME} ), + 'value' => $this->getValueWithDefault( $this->xmlNode ) + ]; } } diff --git a/services/Parser/Nodes/NodeFooter.php b/services/Parser/Nodes/NodeFooter.php index cd89107..cc36b8c 100644 --- a/services/Parser/Nodes/NodeFooter.php +++ b/services/Parser/Nodes/NodeFooter.php @@ -4,13 +4,11 @@ namespace Wikia\PortableInfobox\Parser\Nodes; class NodeFooter extends Node { public function getData() { - $data = []; - $data['value'] = $this->parseWithExternalParser( (string) $this->xmlNode, true ); - return $data; + return [ 'value' => $this->getExternalParser()->parseRecursive( (string)$this->xmlNode ) ]; } public function isEmpty( $data ) { - $links = trim( $data['value'] ); + $links = trim( $data[ 'value' ] ); return empty( $links ); } } diff --git a/services/Parser/Nodes/NodeGroup.php b/services/Parser/Nodes/NodeGroup.php index b6dffbb..49d4af2 100644 --- a/services/Parser/Nodes/NodeGroup.php +++ b/services/Parser/Nodes/NodeGroup.php @@ -1,5 +1,6 @@ externalParser ) { $nodeFactory->setExternalParser( $this->externalParser ); } - $data = []; - $data['value'] = $nodeFactory->getDataFromNodes( $this->xmlNode ); - return $data; + return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ]; } public function isEmpty( $data ) { - foreach ( $data['value'] as $elem ) { - if ( $elem['type'] != 'header' && !($elem['isEmpty']) ) { + foreach ( $data[ 'value' ] as $elem ) { + if ( $elem[ 'type' ] != 'header' && !( $elem[ 'isEmpty' ] ) ) { return false; } } return true; } - - } diff --git a/services/Parser/Nodes/NodeHeader.php b/services/Parser/Nodes/NodeHeader.php index be36e97..6328d79 100644 --- a/services/Parser/Nodes/NodeHeader.php +++ b/services/Parser/Nodes/NodeHeader.php @@ -2,8 +2,8 @@ namespace Wikia\PortableInfobox\Parser\Nodes; class NodeHeader extends Node { - public function getData() { - return [ 'value' => $this->parseWithExternalParser( (string) $this->xmlNode, true ) ]; - } + public function getData() { + return [ 'value' => $this->getExternalParser()->parseRecursive( (string)$this->xmlNode ) ]; + } } diff --git a/services/Parser/Nodes/NodeImage.php b/services/Parser/Nodes/NodeImage.php index 793d0c6..c74fe85 100644 --- a/services/Parser/Nodes/NodeImage.php +++ b/services/Parser/Nodes/NodeImage.php @@ -1,23 +1,24 @@ getValueWithDefault( $this->xmlNode ); - $node['value'] = $this->resolveImageUrl( $imageName ); - $node['alt'] = $this->getValueWithDefault( $this->xmlNode->{self::ALT_TAG_NAME} ); - - return $node; + return [ + 'value' => $this->resolveImageUrl( $this->getValueWithDefault( $this->xmlNode ) ), + 'alt' => $this->getValueWithDefault( $this->xmlNode->{self::ALT_TAG_NAME} ) + ]; } public function resolveImageUrl( $filename ) { global $wgContLang; - $title = \Title::newFromText( \Wikia\PortableInfobox\Helpers\ImageFilenameSanitizer::getInstance() - ->sanitizeImageFileName($filename, $wgContLang), NS_FILE ); + $title = \Title::newFromText( + ImageFilenameSanitizer::getInstance()->sanitizeImageFileName( $filename, $wgContLang ), + NS_FILE + ); if ( $title && $title->exists() ) { return \WikiaFileHelper::getFileFromTitle( $title )->getUrlGenerator()->url(); } else { diff --git a/services/Parser/Nodes/NodeSet.php b/services/Parser/Nodes/NodeSet.php index 4e0a6ac..e725052 100644 --- a/services/Parser/Nodes/NodeSet.php +++ b/services/Parser/Nodes/NodeSet.php @@ -10,14 +10,12 @@ class NodeSet extends Node { if ( $this->externalParser ) { $nodeFactory->setExternalParser( $this->externalParser ); } - $data = []; - $data['value'] = $nodeFactory->getDataFromNodes( $this->xmlNode ); - return $data; + return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ]; } public function isEmpty( $data ) { - foreach ( $data['value'] as $elem ) { - if ( $elem['type'] != 'header' && !($elem['isEmpty']) ) { + foreach ( $data[ 'value' ] as $elem ) { + if ( $elem[ 'type' ] != 'header' && !( $elem[ 'isEmpty' ] ) ) { return false; } } diff --git a/services/Parser/SimpleParser.php b/services/Parser/SimpleParser.php new file mode 100644 index 0000000..1b8fcd3 --- /dev/null +++ b/services/Parser/SimpleParser.php @@ -0,0 +1,13 @@ +parse( $text ); + } + +} \ No newline at end of file diff --git a/services/Parser/XmlParser.php b/services/Parser/XmlParser.php index 1ead179..061bd3b 100644 --- a/services/Parser/XmlParser.php +++ b/services/Parser/XmlParser.php @@ -1,11 +1,11 @@ 'ELEM2', 'lado2' => 'LALALA' - ]); + ] ); $markup = ' @@ -25,22 +25,22 @@ class XmlParserTest extends WikiaBaseTest { '; $data = $parser->getDataFromXmlString( $markup ); // infobox -> comparison -> set -> header - $this->assertTrue( $data[0]['data']['value'][0]['value'][0]['isEmpty'] == false ); + $this->assertFalse( $data[ 0 ][ 'data' ][ 'value' ][ 0 ][ 'data' ][ 'value' ][ 0 ][ 'isEmpty' ] ); // infobox -> comparison -> set -> data { lado1 } - $this->assertTrue( $data[0]['data']['value'][0]['data']['value'][1]['isEmpty'] == true ); + $this->assertTrue( $data[ 0 ][ 'data' ][ 'value' ][ 0 ][ 'data' ][ 'value' ][ 1 ][ 'isEmpty' ] ); // infobox -> comparison -> set -> data { lado2 } - $this->assertTrue( $data[0]['data']['value'][0]['data']['value'][2]['isEmpty'] == false ); + $this->assertFalse( $data[ 0 ][ 'data' ][ 'value' ][ 0 ][ 'data' ][ 'value' ][ 2 ][ 'isEmpty' ] ); // infobox -> comparison -> set - $this->assertTrue( $data[0]['data']['value']['isEmpty'] == false ); + $this->assertFalse( $data[ 0 ][ 'data' ][ 'value' ][ 0 ][ 'isEmpty' ] ); // infobox -> comparison - $this->assertTrue( $data[0]['isEmpty'] == false ); + $this->assertFalse( $data[ 0 ][ 'isEmpty' ] ); } public function testExternalParser() { - $parser = new \Wikia\PortableInfobox\Parser\XmlParser([ + $parser = new \Wikia\PortableInfobox\Parser\XmlParser( [ 'elem2' => 'ELEM2', 'lado2' => 'LALALA' - ]); + ] ); $externalParser = new \Wikia\PortableInfobox\Parser\DummyParser(); $parser->setExternalParser( $externalParser ); $markup = ' @@ -57,7 +57,8 @@ class XmlParserTest extends WikiaBaseTest { '; $data = $parser->getDataFromXmlString( $markup ); - $this->assertTrue( $data[0]['data']['value'] == 'parseRecursive(ABB)' ); - $this->assertTrue( $data[1]['data']['value'][0]['data']['value'][2]['data']['value'] == 'parseRecursive(LALALA)'); + $this->assertEquals( 'parseRecursive(ABB)', $data[ 0 ][ 'data' ][ 'value' ] ); + $this->assertEquals( 'parse(LALALA)', + $data[ 1 ][ 'data' ][ 'value' ][ 0 ][ 'data' ][ 'value' ][ 2 ][ 'data' ][ 'value' ] ); } }