external parser approach changed, tests fixed, refactoring

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';

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 = (string)$xmlNode->{self::DEFAULT_TAG_NAME};
-				$value = $this->parseWithExternalParser( $value, true );
+				$value = $this->getExternalParser()->parseRecursive( $value );
 			}
 		}
 		return $value;
 	}
 
-	protected function getXmlAttribute( \SimpleXMLElement $xmlNode, $attribute )	{
+	protected function getXmlAttribute( \SimpleXMLElement $xmlNode, $attribute ) {
-		if( isset( $xmlNode[ $attribute ] ) )
+		if ( isset( $xmlNode[ $attribute ] ) )
-			return (string) $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 );
 	}
 }

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 = [];
+		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ];
-		$data['value'] = $nodeFactory->getDataFromNodes( $this->xmlNode );
-		return $data;
 	}
 
 	public function isEmpty( $data ) {
-		foreach ( $data['value'] as $group ) {
+		foreach ( $data[ 'value' ] as $group ) {
-			if ( $group['isEmpty'] == false ) {
+			if ( $group[ 'isEmpty' ] == false ) {
 				return false;
 			}
 		}
 		return true;
 	}
-
-
 }

services/Parser/Nodes/NodeData.php

@@ -4,9 +4,9 @@ namespace Wikia\PortableInfobox\Parser\Nodes;
 class NodeData extends Node {
 
 	public function getData() {
-		$data = [];
+		return [
-		$data['label'] = $this->parseWithExternalParser( (string) $this->xmlNode->{self::LABEL_TAG_NAME}, true );
+			'label' => $this->getExternalParser()->parseRecursive( (string) $this->xmlNode->{self::LABEL_TAG_NAME} ),
-		$data['value'] = $this->getValueWithDefault( $this->xmlNode );
+			'value' => $this->getValueWithDefault( $this->xmlNode )
-		return $data;
+		];
 	}
 }

services/Parser/Nodes/NodeFooter.php

@@ -4,13 +4,11 @@ namespace Wikia\PortableInfobox\Parser\Nodes;
 class NodeFooter extends Node {
 
 	public function getData() {
-		$data = [];
+		return [ 'value' => $this->getExternalParser()->parseRecursive( (string)$this->xmlNode ) ];
-		$data['value'] = $this->parseWithExternalParser( (string) $this->xmlNode, true );
-		return $data;
 	}
 
 	public function isEmpty( $data ) {
-		$links = trim( $data['value'] );
+		$links = trim( $data[ 'value' ] );
 		return empty( $links );
 	}
 }

services/Parser/Nodes/NodeGroup.php

@@ -1,5 +1,6 @@
 <?php
 namespace Wikia\PortableInfobox\Parser\Nodes;
+
 use Wikia\PortableInfobox\Parser\XmlParser;
 
 class NodeGroup extends Node {
@@ -9,19 +10,15 @@ class NodeGroup extends Node {
 		if ( $this->externalParser ) {
 			$nodeFactory->setExternalParser( $this->externalParser );
 		}
-		$data = [];
+		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ];
-		$data['value'] = $nodeFactory->getDataFromNodes( $this->xmlNode );
-		return $data;
 	}
 
 	public function isEmpty( $data ) {
-		foreach ( $data['value'] as $elem ) {
+		foreach ( $data[ 'value' ] as $elem ) {
-			if ( $elem['type'] != 'header' && !($elem['isEmpty']) ) {
+			if ( $elem[ 'type' ] != 'header' && !( $elem[ 'isEmpty' ] ) ) {
 				return false;
 			}
 		}
 		return true;
 	}
-
-
 }

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 ) ];
+	}
 }

services/Parser/Nodes/NodeImage.php

@@ -1,23 +1,24 @@
 <?php
 namespace Wikia\PortableInfobox\Parser\Nodes;
 
+use Wikia\PortableInfobox\Helpers\ImageFilenameSanitizer;
+
 class NodeImage extends Node {
 	const ALT_TAG_NAME = 'alt';
 
 	public function getData() {
-		$node = [];
+		return [
-
+			'value' => $this->resolveImageUrl( $this->getValueWithDefault( $this->xmlNode ) ),
-		$imageName = $this->getValueWithDefault( $this->xmlNode );
+			'alt' => $this->getValueWithDefault( $this->xmlNode->{self::ALT_TAG_NAME} )
-		$node['value'] = $this->resolveImageUrl( $imageName );
+		];
-		$node['alt'] = $this->getValueWithDefault( $this->xmlNode->{self::ALT_TAG_NAME} );
-
-		return $node;
 	}
 
 	public function resolveImageUrl( $filename ) {
 		global $wgContLang;
-		$title = \Title::newFromText( \Wikia\PortableInfobox\Helpers\ImageFilenameSanitizer::getInstance()
+		$title = \Title::newFromText(
-			->sanitizeImageFileName($filename, $wgContLang), NS_FILE );
+			ImageFilenameSanitizer::getInstance()->sanitizeImageFileName( $filename, $wgContLang ),
+			NS_FILE
+		);
 		if ( $title && $title->exists() ) {
 			return \WikiaFileHelper::getFileFromTitle( $title )->getUrlGenerator()->url();
 		} else {

services/Parser/Nodes/NodeSet.php

@@ -10,14 +10,12 @@ class NodeSet extends Node {
 		if ( $this->externalParser ) {
 			$nodeFactory->setExternalParser( $this->externalParser );
 		}
-		$data = [];
+		return [ 'value' => $nodeFactory->getDataFromNodes( $this->xmlNode ) ];
-		$data['value'] = $nodeFactory->getDataFromNodes( $this->xmlNode );
-		return $data;
 	}
 
 	public function isEmpty( $data ) {
-		foreach ( $data['value'] as $elem ) {
+		foreach ( $data[ 'value' ] as $elem ) {
-			if ( $elem['type'] != 'header' && !($elem['isEmpty']) ) {
+			if ( $elem[ 'type' ] != 'header' && !( $elem[ 'isEmpty' ] ) ) {
 				return false;
 			}
 		}

services/Parser/SimpleParser.php

@@ -0,0 +1,13 @@
+<?php
+namespace Wikia\PortableInfobox\Parser;
+
+class SimpleParser implements ExternalParser {
+	public function parse( $text ) {
+		return $text;
+	}
+
+	public function parseRecursive( $text ) {
+		return $this->parse( $text );
+	}
+
+}

services/Parser/XmlParser.php

@@ -1,11 +1,11 @@
 <?php
 namespace Wikia\PortableInfobox\Parser;
 
-//interface moved here, because of $wgAutoloadClass issue
+////interface moved here, because of $wgAutoloadClass issue
-interface ExternalParser {
+//interface ExternalParser {
-	public function parse( $text );
+//	public function parse( $text );
-	public function parseRecursive( $text );
+//	public function parseRecursive( $text );
-}
+//}
 
 class XmlParser {
 

tests/XmlParserTest.php

@@ -8,10 +8,10 @@ class XmlParserTest extends WikiaBaseTest {
 	}
 
 	public function testIsEmpty() {
-		$parser = new \Wikia\PortableInfobox\Parser\XmlParser([
+		$parser = new \Wikia\PortableInfobox\Parser\XmlParser( [
 			'elem2' => 'ELEM2',
 			'lado2' => 'LALALA'
-		]);
+		] );
 		$markup = '
 			<infobox>
 				<comparison>
@@ -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 {
 			</infobox>
 		';
 		$data = $parser->getDataFromXmlString( $markup );
-		$this->assertTrue( $data[0]['data']['value'] == 'parseRecursive(ABB)' );
+		$this->assertEquals( 'parseRecursive(ABB)', $data[ 0 ][ 'data' ][ 'value' ] );
-		$this->assertTrue( $data[1]['data']['value'][0]['data']['value'][2]['data']['value'] == 'parseRecursive(LALALA)');
+		$this->assertEquals( 'parse(LALALA)',
+			$data[ 1 ][ 'data' ][ 'value' ][ 0 ][ 'data' ][ 'value' ][ 2 ][ 'data' ][ 'value' ] );
 	}
 }