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

Reply via email to