From e4afb0c69794d43e43a016d29ecfb926366401ab Mon Sep 17 00:00:00 2001 From: Garth Webb Date: Tue, 19 May 2015 11:20:44 -0700 Subject: [PATCH] SOC-808 Merge part 1 of 2 --- PortableInfobox.setup.php | 1 + services/Helpers/SimpleXmlUtil.php | 37 +++++++++++++++ services/Parser/Nodes/Node.php | 16 +++---- services/Parser/Nodes/NodeData.php | 6 ++- services/Parser/Nodes/NodeFooter.php | 6 ++- services/Parser/Nodes/NodeHeader.php | 4 +- tests/NodeTest.php | 70 ++++++++++++++++++++++++++++ tests/ParserNodesTest.php | 52 --------------------- tests/SimpleXmlUtilTest.php | 56 ++++++++++++++++++++++ 9 files changed, 182 insertions(+), 66 deletions(-) create mode 100644 services/Helpers/SimpleXmlUtil.php create mode 100644 tests/NodeTest.php create mode 100644 tests/SimpleXmlUtilTest.php diff --git a/PortableInfobox.setup.php b/PortableInfobox.setup.php index 3fab3cd..e37b187 100644 --- a/PortableInfobox.setup.php +++ b/PortableInfobox.setup.php @@ -41,6 +41,7 @@ foreach ( $wgInfoboxParserNodes as $parserNode ) { // helpers $wgAutoloadClasses[ 'Wikia\PortableInfobox\Helpers\ImageFilenameSanitizer' ] = $dir . 'services/Helpers/ImageFilenameSanitizer.php'; +$wgAutoloadClasses[ 'Wikia\PortableInfobox\Helpers\SimpleXmlUtil' ] = $dir . 'services/Helpers/SimpleXmlUtil.php'; // controller classes $wgAutoloadClasses[ 'PortableInfoboxParserTagController' ] = $dir . 'controllers/PortableInfoboxParserTagController.class.php'; diff --git a/services/Helpers/SimpleXmlUtil.php b/services/Helpers/SimpleXmlUtil.php new file mode 100644 index 0000000..b7e4170 --- /dev/null +++ b/services/Helpers/SimpleXmlUtil.php @@ -0,0 +1,37 @@ +childNodes as $child ) { + $innerXML .= $child->ownerDocument->saveXML( $child ); + } + } + return $innerXML; + } +} diff --git a/services/Parser/Nodes/Node.php b/services/Parser/Nodes/Node.php index 5b8565d..03a4141 100644 --- a/services/Parser/Nodes/Node.php +++ b/services/Parser/Nodes/Node.php @@ -46,16 +46,10 @@ class Node { return [ 'value' => (string)$this->xmlNode ]; } + //note that a '0' value cannot be treated like a null public function isEmpty( $data ) { - return !( isset( $data[ 'value' ] ) ) || empty( $data[ 'value' ] ); - } - - protected function getInnerXML( \SimpleXMLElement $node ) { - $innerXML= ''; - foreach ( dom_import_simplexml( $node )->childNodes as $child ) { - $innerXML .= $child->ownerDocument->saveXML( $child ); - } - return $innerXML; + $value = $data[ 'value' ]; + return !( isset( $value ) ) || (empty( $value ) && $value != '0'); } protected function getValueWithDefault( \SimpleXMLElement $xmlNode ) { @@ -71,7 +65,9 @@ class Node { * We should not parse it's contents as XML but return pure text in order to let MediaWiki Parser * parse it. */ - $value = $this->getInnerXML( $xmlNode->{self::DEFAULT_TAG_NAME} ); + $value = \Wikia\PortableInfobox\Helpers\SimpleXmlUtil::getInstance()->getInnerXML( + $xmlNode->{self::DEFAULT_TAG_NAME} + ); $value = $this->getExternalParser()->parseRecursive( $value ); } } diff --git a/services/Parser/Nodes/NodeData.php b/services/Parser/Nodes/NodeData.php index 0f0d266..a529f48 100644 --- a/services/Parser/Nodes/NodeData.php +++ b/services/Parser/Nodes/NodeData.php @@ -5,7 +5,11 @@ class NodeData extends Node { public function getData() { return [ - 'label' => $this->getExternalParser()->parseRecursive( $this->getInnerXML( $this->xmlNode->{self::LABEL_TAG_NAME} ) ), + 'label' => $this->getExternalParser()->parseRecursive( + \Wikia\PortableInfobox\Helpers\SimpleXmlUtil::getInstance()->getInnerXML( + $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 7f1b6c1..f3332dd 100644 --- a/services/Parser/Nodes/NodeFooter.php +++ b/services/Parser/Nodes/NodeFooter.php @@ -4,11 +4,13 @@ namespace Wikia\PortableInfobox\Parser\Nodes; class NodeFooter extends Node { public function getData() { - return [ 'value' => $this->getExternalParser()->parseRecursive( $this->getInnerXML( $this->xmlNode ) ) ]; + return [ 'value' => $this->getExternalParser()->parseRecursive( + \Wikia\PortableInfobox\Helpers\SimpleXmlUtil::getInstance()->getInnerXML( $this->xmlNode ) + ) ]; } public function isEmpty( $data ) { - $links = trim( $data[ 'value' ] ); + $links = trim( $data['value'] ); return empty( $links ); } } diff --git a/services/Parser/Nodes/NodeHeader.php b/services/Parser/Nodes/NodeHeader.php index 991daf3..50713c6 100644 --- a/services/Parser/Nodes/NodeHeader.php +++ b/services/Parser/Nodes/NodeHeader.php @@ -4,6 +4,8 @@ namespace Wikia\PortableInfobox\Parser\Nodes; class NodeHeader extends Node { public function getData() { - return [ 'value' => $this->getExternalParser()->parseRecursive( $this->getInnerXML( $this->xmlNode ) ) ]; + return [ 'value' => $this->getExternalParser()->parseRecursive( + \Wikia\PortableInfobox\Helpers\SimpleXmlUtil::getInstance()->getInnerXML( $this->xmlNode ) + ) ]; } } diff --git a/tests/NodeTest.php b/tests/NodeTest.php new file mode 100644 index 0000000..c1c4f35 --- /dev/null +++ b/tests/NodeTest.php @@ -0,0 +1,70 @@ +setupFile = dirname( __FILE__ ) . '/../PortableInfobox.setup.php'; + parent::setUp(); + } + + /** + * @dataProvider testIsEmptyDataProvider + */ + public function testIsEmpty( $season, $expectedOutput ) { + $string = ''; + $xml = simplexml_load_string( $string ); + + $node = new Wikia\PortableInfobox\Parser\Nodes\NodeData( $xml, [ 'season' => $season ] ); + $nodeData = $node->getData(); + $this->assertTrue( $node->isEmpty( $nodeData ) == $expectedOutput ); + } + + public function testIsEmptyDataProvider() { + return [ + [ + 'season' => '0', + 'expectedOutput' => false + ], + [ + 'season' => '1', + 'expectedOutput' => false + ], + [ + 'season' => 'one', + 'expectedOutput' => false + ], + [ + 'season' => 'null', + 'expectedOutput' => false + ], + [ + 'season' => 'false', + 'expectedOutput' => false + ], + [ + 'season' => ' ', + 'expectedOutput' => false + ], + [ + 'season' => null, + 'expectedOutput' => true + ], + [ + 'season' => [], + 'expectedOutput' => true + ], + [ + 'season' => '', + 'expectedOutput' => true + ], + [ + 'season' => 5, + 'expectedOutput' => false + ], + [ + 'season' => 0, + 'expectedOutput' => false + ] + ]; + } +} diff --git a/tests/ParserNodesTest.php b/tests/ParserNodesTest.php index 2f8ba65..72ede5b 100644 --- a/tests/ParserNodesTest.php +++ b/tests/ParserNodesTest.php @@ -41,22 +41,6 @@ class PortableInfoboxParserNodesTest extends WikiaBaseTest { $this->assertTrue( $nodeDefault->getData()[ 'alt' ] == 'default-alt', 'default alt' ); } - /** - * @dataProvider testNodeImageVariableReplaceProvider - * @todo rethink this test cause after changes to NodeImage it doesn't make sense in this form - */ -// public function testNodeImageVariableReplace( $xmlString, $data, $expValue ) { -// $xml = simplexml_load_string($xmlString); -// $node = $this->getMockBuilder( 'Wikia\PortableInfobox\Parser\Nodes\NodeImage' ) -// ->setConstructorArgs( [ $xml, $data ] ) -// ->setMethods( [ 'resolveImageUrl' ] ) -// ->getMock(); -// $node->expects( $this->any() )->method( 'resolveImageUrl' )->with( $this->equalTo( $expValue ) ); -// $externalParser = new \Wikia\PortableInfobox\Parser\DummyParser(); -// $node->setExternalParser( $externalParser ); -// $node->getData(); -// } - public function testNodeHeader() { $string = '
Comandantes
'; $xml = simplexml_load_string( $string ); @@ -116,40 +100,4 @@ class PortableInfoboxParserNodesTest extends WikiaBaseTest { $this->assertTrue( $data[ 'value' ][ 0 ]['data']['value'][ 1 ][ 'type' ] == 'data' ); $this->assertTrue( $data[ 'value' ][ 0 ]['data']['value'][ 2 ][ 'data' ][ 'value' ] == 2 ); } - - public function testNodeImageVariableReplaceProvider() { - return [ - [ - 'default-alt', - [ - 'image2' => 'Wiki-image' - ], - 'Wiki-image', - 'Regular filename should be untouched' - ], - [ - 'default-alt', - [ - 'image2' => '[[Wiki-image]]' - ], - '[[Wiki-image]]', - 'Link to filename should be untouched' - ], - [ - 'default-alt', - [ - 'image2' => '[[File:Wiki-image]]' - ], - '[[File:Wiki-image]]', - 'File invocation in params should be untouched' - ], - [ - '[[File:Wiki-image]]default-alt', - [ ], - 'replaceVariables([[File:Wiki-image]])', - 'File invocation in default should invoke replace variables' - ], - - ]; - } } diff --git a/tests/SimpleXmlUtilTest.php b/tests/SimpleXmlUtilTest.php new file mode 100644 index 0000000..1990fea --- /dev/null +++ b/tests/SimpleXmlUtilTest.php @@ -0,0 +1,56 @@ +setupFile = dirname( __FILE__ ) . '/../PortableInfobox.setup.php'; + parent::setUp(); + + $this->simpleXmlUtil = \Wikia\PortableInfobox\Helpers\SimpleXmlUtil::getInstance(); + } + + /** + * @dataProvider testGetInnerXMLDataProvider + */ + public function testGetInnerXML( $xmlString, $expValue, $message ) { + $xml = simplexml_load_string( $xmlString ); + $this->assertEquals( $expValue, $this->simpleXmlUtil->getInnerXML( $xml ), $message ); + } + + public function testGetInnerXMLDataProvider() { + return [ + [ + 'TestFile.png', + 'TestFile.png', + 'Valid gallery tag in default' + ], + [ + 'Test ', + 'Test ', + 'Text + XML' + ], + [ + '', + '', + 'Valid self-closing tag' + + ], + [ + '', + '', + 'Invalid inner XML' + ], + [ + '', + '', + 'Invalid inner XML' + ], + [ + '', + '', + 'No inner XML' + ] + ]; + } +}