Mwjames has uploaded a new change for review. https://gerrit.wikimedia.org/r/59057
Change subject: Decouple responsibility, move value factory out of ParserData ...................................................................... Decouple responsibility, move value factory out of ParserData Inject datavalue dependency instead of doing value factoring within the ParserData class itself. Former addPropertyValueString() was responsible for creating a value object, error handling and attachement to the semantic container. Splitting the task will help the refactoring of ParserExtensions class. Change-Id: I0811692033ec7b6f69e4e78efc25d3728dbf1a01 --- M includes/ParserData.php M includes/SMW_DataValueFactory.php M includes/parserhooks/SetParserFunction.php A tests/phpunit/includes/DataValueFactoryTest.php M tests/phpunit/includes/DiffDataTest.php M tests/phpunit/includes/ParserDataTest.php 6 files changed, 210 insertions(+), 36 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/SemanticMediaWiki refs/changes/57/59057/1 diff --git a/includes/ParserData.php b/includes/ParserData.php index cad9e03..c4d2284 100644 --- a/includes/ParserData.php +++ b/includes/ParserData.php @@ -10,10 +10,10 @@ use MWException; use SMWStore; +use SMWDataValue; use SMWDIWikiPage; use SMWPropertyValue; use SMWSemanticData; -use SMWDataValueFactory; use SMWDIProperty; use SMWDIBlob; use SMWDIBoolean; @@ -326,42 +326,42 @@ } /** - * Add value string to the instantiated semanticData container + * This method adds a data value to the semantic data container + * + * @par Example: + * @code + * $parserData = new SMW\ParserData( + * $parser->getTitle(), + * $parser->getOutput(), + * $settings; + * ) + * + * $dataValue = SMWDataValueFactory::newPropertyValue( $userProperty, $userValue ) + * $parserData->addPropertyValue( $dataValue ) + * @endcode * * @since 1.9 * - * @param string $propertyName - * @param string $valueString - * - * @throws MWException + * @param SMWDataValue $dataValue */ - public function addPropertyValueString( $propertyName, $valueString ) { - if ( !( $this->semanticData instanceof SMWSemanticData ) ) { - throw new MWException( 'The semantic data container is not initialized' ); - } - + public function addPropertyValue( SMWDataValue $dataValue ) { wfProfileIn( __METHOD__ ); - $propertyDv = SMWPropertyValue::makeUserProperty( $propertyName ); - $propertyDi = $propertyDv->getDataItem(); - - if ( $propertyDi instanceof \SMWDIProperty && !$propertyDi->isInverse() ) { - $valueDv = SMWDataValueFactory::newPropertyObjectValue( $propertyDi, $valueString, - false, $this->semanticData->getSubject() ); - $this->semanticData->addPropertyObjectValue( $propertyDi, $valueDv->getDataItem() ); - - // Take note of the error for storage (do this here and not in storage, thus avoiding duplicates). - if ( !$valueDv->isValid() ) { + if ( $dataValue->getProperty() instanceof SMWDIProperty ) { + if ( !$dataValue->isValid() ) { $this->semanticData->addPropertyObjectValue( new SMWDIProperty( SMWDIProperty::TYPE_ERROR ), - $propertyDi->getDiWikiPage() + $dataValue->getProperty()->getDiWikiPage() ); - $this->setError( $valueDv->getErrors() ); + $this->setError( $dataValue->getErrors() ); + } else { + $this->semanticData->addPropertyObjectValue( + $dataValue->getProperty(), + $dataValue->getDataItem() + ); } - } else if ( $propertyDi instanceof \SMWDIProperty && $propertyDi->isInverse() ) { - $this->setError( array( wfMessage( 'smw_noinvannot' )->inContentLanguage()->text() ) ); } else { - $this->setError( array( wfMessage( 'smw-property-name-invalid', $propertyName )->inContentLanguage()->text() ) ); + $this->setError( $dataValue->getErrors() ); } wfProfileOut( __METHOD__ ); diff --git a/includes/SMW_DataValueFactory.php b/includes/SMW_DataValueFactory.php index 9694d97..f34f20b 100644 --- a/includes/SMW_DataValueFactory.php +++ b/includes/SMW_DataValueFactory.php @@ -183,6 +183,62 @@ } /** + * This factory method returns a data value object from a given property, + * value string. It is intended to be used on user input to allow to + * turn a property and value strings into a data value object. + * + * @since 1.9 + * + * @param string $propertyName property string + * @param string $valueString user value string + * @param mixed $caption user-defined caption + * @param SMWDIWikiPage|null $contextPage context for parsing the value string + * + * @return SMWDataValue + */ + public static function newPropertyValue( $propertyName, $valueString, + $caption = false, SMWDIWikiPage $contextPage = null ) { + + wfProfileIn( __METHOD__ ); + + $propertyDV = SMWPropertyValue::makeUserProperty( $propertyName ); + + if ( !$propertyDV->isValid() ) { + wfProfileOut( __METHOD__ ); + return $propertyDV; + } + + $propertyDI = $propertyDV->getDataItem(); + + if ( $propertyDI instanceof SMWDIError ) { + wfProfileOut( __METHOD__ ); + return $propertyDV; + } + + if ( $propertyDI instanceof SMWDIProperty && !$propertyDI->isInverse() ) { + $dataValue = self::newPropertyObjectValue( + $propertyDI, + $valueString, + $caption, + $contextPage + ); + } else if ( $propertyDI instanceof SMWDIProperty && $propertyDI->isInverse() ) { + $dataValue = new SMWErrorValue( $propertyDV->getPropertyTypeID(), + wfMessage( 'smw_noinvannot' )->inContentLanguage()->text(), + $valueString, $caption + ); + } else { + $dataValue = new SMWErrorValue( $propertyDV->getPropertyTypeID(), + wfMessage( 'smw-property-name-invalid', $propertyName )->inContentLanguage()->text(), + $valueString, $caption + ); + } + + wfProfileOut( __METHOD__ ); + return $dataValue; + } + + /** * Gather all available datatypes and label<=>id<=>datatype * associations. This method is called before most methods of this * factory. diff --git a/includes/parserhooks/SetParserFunction.php b/includes/parserhooks/SetParserFunction.php index 23447dc..837c590 100644 --- a/includes/parserhooks/SetParserFunction.php +++ b/includes/parserhooks/SetParserFunction.php @@ -3,6 +3,7 @@ namespace SMW; use Parser; +use SMWDataValueFactory; /** * {{#set}} parser function @@ -74,7 +75,12 @@ // Add value strings foreach ( $parameters->toArray() as $property => $values ){ foreach ( $values as $value ) { - $this->parserData->addPropertyValueString( $property, $value ); + $this->parserData->addPropertyValue( + SMWDataValueFactory::newPropertyValue( + $property, + $value + ) + ); } } diff --git a/tests/phpunit/includes/DataValueFactoryTest.php b/tests/phpunit/includes/DataValueFactoryTest.php new file mode 100644 index 0000000..092fbae --- /dev/null +++ b/tests/phpunit/includes/DataValueFactoryTest.php @@ -0,0 +1,93 @@ +<?php + +namespace SMW\Test; + +use SMWDataValueFactory; +use SMWDataItem; + +/** + * Tests for the SMWDataValueFactory class + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + * @since 1.9 + * + * @ingroup SMW + * @ingroup Test + * + * @group SMW + * @group SMWExtension + * + * @licence GNU GPL v2+ + * @author mwjames + */ + +/** + * Tests for the SMWDataValueFactory class + * + * @ingroup SMW + * @ingroup Test + */ +class DataValueFactoryTest extends \MediaWikiTestCase { + + /** + * DataProvider + * + * @return array + */ + public function getPropertyValueDataProvider() { + return array( + array( 'Foo' , 'Bar' , 'Bar' , 'SMWDataValue' ), + array( 'Foo' , 'Bar[[ Foo ]]' , 'Bar[[ Foo ]]' , 'SMWDataValue' ), + array( 'Foo' , '9001' , '9001' , 'SMWDataValue' ), + array( 'Foo' , 1001 , '1001' , 'SMWDataValue' ), + array( 'Foo' , '-%&$*' , '-%&$*' , 'SMWDataValue' ), + array( 'Foo' , '_Bar' , 'Bar' , 'SMWDataValue' ), + array( 'Foo' , 'bar' , 'Bar' , 'SMWDataValue' ), + array( '-Foo' , 'Bar' , '' , 'SMWErrorValue' ), + array( '_Foo' , 'Bar' , '' , 'SMWPropertyValue' ), + ); + } + + /** + * @dataProvider getPropertyValueDataProvider + * + * @see SMWDataValueFactory::addPropertyValue + * @since 1.9 + * + * @param $propertyName + * @param $value + * @param $expectedValue + * @param $expectedInstance + */ + public function testAddPropertyValue( $propertyName, $value, $expectedValue, $expectedInstance ) { + $dataValue = SMWDataValueFactory::newPropertyValue( $propertyName, $value ); + + // Check the returned instance + $this->assertInstanceOf( $expectedInstance , $dataValue ); + + if ( $dataValue->getErrors() === array() ){ + $this->assertInstanceOf( 'SMWDIProperty', $dataValue->getProperty() ); + $this->assertContains( $propertyName, $dataValue->getProperty()->getLabel() ); + if ( $dataValue->getDataItem()->getDIType() === SMWDataItem::TYPE_WIKIPAGE ){ + $this->assertEquals( $expectedValue, $dataValue->getWikiValue() ); + } + } else { + $this->assertInternalType( 'array', $dataValue->getErrors() ); + } + } +} diff --git a/tests/phpunit/includes/DiffDataTest.php b/tests/phpunit/includes/DiffDataTest.php index ad1809e..fa533e4 100644 --- a/tests/phpunit/includes/DiffDataTest.php +++ b/tests/phpunit/includes/DiffDataTest.php @@ -6,6 +6,7 @@ use SMW\Settings; use SMW\ParserData; use SMWDIWikiPage; +use SMWDataValueFactory; use ReflectionClass; use ParserOutput; @@ -94,6 +95,18 @@ } /** + * Helper method that returns a SMWDataValue object + * + * @return SMWDataValue + */ + private function getDataValue( $propertyName, $value ){ + return SMWDataValueFactory::newPropertyValue( + $propertyName, + $value + ); + } + + /** * Helper method that returns a ParserData object * * @return SMW\ParserData @@ -166,7 +179,7 @@ // Add semantic data $prop = 'Bar'; - $parserData->addPropertyValueString( $prop, '9001' ); + $parserData->addPropertyValue( $this->getDataValue( $prop, '9001' ) ); $parserData->updateOutput(); // Get stored data @@ -198,7 +211,7 @@ // Add semantic data $prop = 'Bar'; - $parserData->addPropertyValueString( $prop, '9001' ); + $parserData->addPropertyValue( $this->getDataValue( $prop, '9001' ) ); $parserData->updateOutput(); // Get stored data @@ -226,7 +239,7 @@ // Add semantic data $prop = 'Bar'; - $parserData->addPropertyValueString( $prop, '9001' ); + $parserData->addPropertyValue( $this->getDataValue( $prop, '9001' ) ); $parserData->updateOutput(); // Get stored data diff --git a/tests/phpunit/includes/ParserDataTest.php b/tests/phpunit/includes/ParserDataTest.php index 8729355..44055d7 100644 --- a/tests/phpunit/includes/ParserDataTest.php +++ b/tests/phpunit/includes/ParserDataTest.php @@ -95,7 +95,7 @@ * * @return array */ - public function getPropertyValueStringDataProvider() { + public function getPropertyValueDataProvider() { // property, value, errorCount return array( @@ -106,19 +106,25 @@ } /** - * @covers SMW\ParserData::addPropertyValueString + * @dataProvider getPropertyValueDataProvider * - * @dataProvider getPropertyValueStringDataProvider - * + * @see SMW\ParserData::addPropertyValue * @since 1.9 * * @param $propertyName * @param $value * @param $error */ - public function testAddPropertyValueString( $propertyName, $value, $error ) { + public function testAddPropertyValue( $propertyName, $value, $error ) { $instance = $this->getInstance( 'Foo', $this->getParserOutput() ); - $instance->addPropertyValueString( $propertyName, $value ); + + // Values + $instance->addPropertyValue( + SMWDataValueFactory::newPropertyValue( + $propertyName, + $value + ) + ); // Check the returned instance $this->assertInstanceOf( 'SMWSemanticData', $instance->getData() ); -- To view, visit https://gerrit.wikimedia.org/r/59057 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I0811692033ec7b6f69e4e78efc25d3728dbf1a01 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/SemanticMediaWiki Gerrit-Branch: master Gerrit-Owner: Mwjames <jamesin.hongkon...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits