Jeroen De Dauw has submitted this change and it was merged.
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/ConceptParserFunction.php
M includes/parserhooks/RecurringEventsParserFunction.php
M includes/parserhooks/SetParserFunction.php
M includes/parserhooks/SubobjectParserFunction.php
A tests/phpunit/includes/DataValueFactoryTest.php
M tests/phpunit/includes/ParserDataTest.php
8 files changed, 199 insertions(+), 39 deletions(-)
Approvals:
Jeroen De Dauw: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/ParserData.php b/includes/ParserData.php
index e074760..4f3d3f7 100644
--- a/includes/ParserData.php
+++ b/includes/ParserData.php
@@ -9,10 +9,9 @@
use Job;
use SMWStore;
+use SMWDataValue;
use SMWDIWikiPage;
-use SMWPropertyValue;
use SMWSemanticData;
-use SMWDataValueFactory;
use SMWDIProperty;
use SMWDIBlob;
use SMWDIBoolean;
@@ -257,7 +256,7 @@
*
* @return array
*/
- public function setError( array $errors ) {
+ public function addError( array $errors ) {
return $this->errors = array_merge ( $errors, $this->errors );
}
@@ -341,42 +340,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->addError( $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->addError( $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/ConceptParserFunction.php
b/includes/parserhooks/ConceptParserFunction.php
index 6129bff..765fa2d 100644
--- a/includes/parserhooks/ConceptParserFunction.php
+++ b/includes/parserhooks/ConceptParserFunction.php
@@ -164,7 +164,7 @@
);
// Store query data to the ParserOutput
- $this->parserData->setError( $this->query->getErrors() );
+ $this->parserData->addError( $this->query->getErrors() );
// Update ParserOutput
$this->parserData->updateOutput();
diff --git a/includes/parserhooks/RecurringEventsParserFunction.php
b/includes/parserhooks/RecurringEventsParserFunction.php
index ff8a8a4..58f7d15 100644
--- a/includes/parserhooks/RecurringEventsParserFunction.php
+++ b/includes/parserhooks/RecurringEventsParserFunction.php
@@ -72,7 +72,7 @@
// Get recurring events
$events = new RecurringEvents( $parameters->toArray(),
$this->getSettings() );
- $this->parserData->setError( $events->getErrors() );
+ $this->parserData->addError( $events->getErrors() );
foreach ( $events->getDates() as $date_str ) {
@@ -108,7 +108,7 @@
);
// Collect possible errors that occurred during
processing
- $this->parserData->setError(
$this->subobject->getErrors() );
+ $this->parserData->addError(
$this->subobject->getErrors() );
}
// Update ParserOutput
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/includes/parserhooks/SubobjectParserFunction.php
b/includes/parserhooks/SubobjectParserFunction.php
index c551ca4..9f48767 100644
--- a/includes/parserhooks/SubobjectParserFunction.php
+++ b/includes/parserhooks/SubobjectParserFunction.php
@@ -126,7 +126,7 @@
$this->subobject->getContainer()
);
- $this->parserData->setError( $this->subobject->getErrors() );
+ $this->parserData->addError( $this->subobject->getErrors() );
// Update ParserOutput
$this->parserData->updateOutput();
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/ParserDataTest.php
b/tests/phpunit/includes/ParserDataTest.php
index 104d2af..414e9fa 100644
--- a/tests/phpunit/includes/ParserDataTest.php
+++ b/tests/phpunit/includes/ParserDataTest.php
@@ -89,7 +89,7 @@
*
* @return array
*/
- public function getPropertyValueStringDataProvider() {
+ public function getPropertyValueDataProvider() {
// property, value, errorCount
return array(
@@ -100,19 +100,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: merged
Gerrit-Change-Id: I0811692033ec7b6f69e4e78efc25d3728dbf1a01
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/SemanticMediaWiki
Gerrit-Branch: master
Gerrit-Owner: Mwjames <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Mwjames <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
