jenkins-bot has submitted this change and it was merged.
Change subject: Allow callbacks in factories for formatters/parsers.
......................................................................
Allow callbacks in factories for formatters/parsers.
Using callbacks in ValueFormatterFactory and ValueParserFactory
allows builders to inject services when creating formatters resp. builders.
Change-Id: I31b8786f536ec732d53ca33f0859cc6163c7e779
---
M DataValuesCommon/src/ValueFormatters/ValueFormatterFactory.php
M DataValuesCommon/src/ValueParsers/ApiParseValue.php
M DataValuesCommon/src/ValueParsers/ValueParserFactory.php
M DataValuesCommon/tests/ValueFormatters/ValueFormatterFactoryTest.php
M DataValuesCommon/tests/ValueParsers/ValueParserFactoryTest.php
5 files changed, 147 insertions(+), 61 deletions(-)
Approvals:
Tobias Gritschacher: Looks good to me, approved
jenkins-bot: Verified
diff --git a/DataValuesCommon/src/ValueFormatters/ValueFormatterFactory.php
b/DataValuesCommon/src/ValueFormatters/ValueFormatterFactory.php
index f569ed1..eeee42a 100644
--- a/DataValuesCommon/src/ValueFormatters/ValueFormatterFactory.php
+++ b/DataValuesCommon/src/ValueFormatters/ValueFormatterFactory.php
@@ -2,6 +2,10 @@
namespace ValueFormatters;
+use InvalidArgumentException;
+use LogicException;
+use OutOfBoundsException;
+
/**
* Factory for creating ValueFormatter objects.
*
@@ -12,29 +16,38 @@
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
+ * @author Daniel Kinzler
*/
class ValueFormatterFactory {
/**
- * Maps parser id to ValueFormatter class.
+ * Maps formatter id to ValueFormatter class or builder callback.
*
* @since 0.1
*
- * @var ValueFormatter[]
+ * @var array
*/
protected $formatters = array();
/**
* @since 0.1
*
- * @param string[] $valueFormatters
+ * @param string|callable[] $valueFormatters An associative array
mapping formatter ids to
+ * class names or callable builders.
+ *
+ * @throws InvalidArgumentException
*/
public function __construct( array $valueFormatters ) {
- foreach ( $valueFormatters as $formatterId => $formatterClass )
{
- assert( is_string( $formatterId ) );
- assert( is_string( $formatterClass ) );
+ foreach ( $valueFormatters as $formatterId => $formatterBuilder
) {
+ if ( !is_string( $formatterId ) ) {
+ throw new InvalidArgumentException( 'Formatter
id needs to be a string' );
+ }
- $this->formatters[$formatterId] = $formatterClass;
+ if ( !is_string( $formatterBuilder ) && !is_callable(
$formatterBuilder ) ) {
+ throw new InvalidArgumentException( 'Formatter
class needs to be a class name or callable' );
+ }
+
+ $this->formatters[$formatterId] = $formatterBuilder;
}
}
@@ -50,16 +63,21 @@
}
/**
- * Returns class of the ValueFormatter with the provided id or null if
there is no such ValueFormatter.
+ * Returns the formatter builder (class name or callable) for
$formatterId, or null if
+ * no builder was registered for that id.
*
* @since 0.1
*
* @param string $formatterId
*
- * @return string|null
+ * @return string|callable|null
*/
- public function getFormatterClass( $formatterId ) {
- return array_key_exists( $formatterId, $this->formatters ) ?
$this->formatters[$formatterId] : null;
+ public function getFormatterBuilder( $formatterId ) {
+ if ( array_key_exists( $formatterId, $this->formatters ) ) {
+ return $this->formatters[$formatterId];
+ }
+
+ return null;
}
/**
@@ -68,20 +86,43 @@
* @since 0.1
*
* @param string $formatterId
- * @param FormatterOptions $options
+ * @param FormatterOptions $formatterOptions
*
- * @return ValueFormatter|null
+ * @throws OutOfBoundsException If no formatter was registered for
$formatterId
+ * @return ValueFormatter
*/
- public function newFormatter( $formatterId, FormatterOptions $options )
{
+ public function newFormatter( $formatterId, FormatterOptions
$formatterOptions ) {
if ( !array_key_exists( $formatterId, $this->formatters ) ) {
- return null;
+ throw new OutOfBoundsException( "No builder registered
for formatter ID $formatterId" );
}
- $instance = new $this->formatters[$formatterId]( $options );
+ $builder = $this->formatters[$formatterId];
+ $formatter = $this->instantiateFormatter( $builder,
$formatterOptions );
- assert( $instance instanceof ValueFormatter );
+ return $formatter;
+ }
- return $instance;
+ /**
+ * @param string|callable $builder Either a classname of an
implementation of ValueFormatter,
+ * or a callable that returns a ValueFormatter. $options will be
passed to the constructor
+ * or callable, respectively.
+ * @param FormatterOptions $options
+ *
+ * @throws LogicException if the builder did not create a ValueFormatter
+ * @return ValueFormatter
+ */
+ private function instantiateFormatter( $builder, FormatterOptions
$options ) {
+ if ( is_string( $builder ) ) {
+ $formatter = new $builder( $options );
+ } else {
+ $formatter = call_user_func( $builder, $options );
+ }
+
+ if ( !( $formatter instanceof Valueformatter ) ) {
+ throw new LogicException( "Invalid formatter builder,
did not create an instance of ValueFormatter." );
+ }
+
+ return $formatter;
}
}
diff --git a/DataValuesCommon/src/ValueParsers/ApiParseValue.php
b/DataValuesCommon/src/ValueParsers/ApiParseValue.php
index 655330f..0d324eb 100644
--- a/DataValuesCommon/src/ValueParsers/ApiParseValue.php
+++ b/DataValuesCommon/src/ValueParsers/ApiParseValue.php
@@ -6,6 +6,7 @@
use DataValues\DataValue;
use LogicException;
use MWException;
+use OutOfBoundsException;
/**
* API module for using value parsers.
@@ -65,10 +66,10 @@
$params = $this->extractRequestParams();
$options = $this->getOptionsObject( $params['options'] );
- $parser = $this->getFactory()->newParser( $params['parser'],
$options );
- // Paranoid check, should never fail as we only accept
registered parsers for the parser parameter.
- if ( $parser === null ) {
+ try {
+ $parser = $this->getFactory()->newParser(
$params['parser'], $options );
+ } catch ( OutOfBoundsException $ex ) {
throw new LogicException( 'Could not obtain a
ValueParser instance' );
}
diff --git a/DataValuesCommon/src/ValueParsers/ValueParserFactory.php
b/DataValuesCommon/src/ValueParsers/ValueParserFactory.php
index fbc431a..e6bf24e 100644
--- a/DataValuesCommon/src/ValueParsers/ValueParserFactory.php
+++ b/DataValuesCommon/src/ValueParsers/ValueParserFactory.php
@@ -3,6 +3,8 @@
namespace ValueParsers;
use InvalidArgumentException;
+use LogicException;
+use OutOfBoundsException;
/**
* Factory for creating ValueParser objects.
@@ -14,11 +16,12 @@
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
+ * @author Daniel Kinzler
*/
class ValueParserFactory {
/**
- * Maps parser id to ValueParser class or instance.
+ * Maps parser id to ValueParser class or builder callback.
*
* @since 0.1
*
@@ -29,19 +32,22 @@
/**
* @since 0.1
*
- * @param array $valueParsers
+ * @param string|callable[] $valueParsers An associative array mapping
parser ids to
+ * class names or callable builders.
+ *
+ * @throws InvalidArgumentException
*/
public function __construct( array $valueParsers ) {
- foreach ( $valueParsers as $parserId => $parserClass ) {
+ foreach ( $valueParsers as $parserId => $parserBuilder ) {
if ( !is_string( $parserId ) ) {
throw new InvalidArgumentException( 'Parser id
needs to be a string' );
}
- if ( !is_string( $parserClass ) ) {
- throw new InvalidArgumentException( 'Parser
class needs to be a string' );
+ if ( !is_string( $parserBuilder ) && !is_callable(
$parserBuilder ) ) {
+ throw new InvalidArgumentException( 'Parser
class needs to be a class name or callable' );
}
- $this->parsers[$parserId] = $parserClass;
+ $this->parsers[$parserId] = $parserBuilder;
}
}
@@ -57,17 +63,18 @@
}
/**
- * Returns class of the ValueParser with the provided id or null if
there is no such ValueParser.
+ * Returns the parser builder (class name or callable) for $parserId,
or null if
+ * no builder was registered for that id.
*
* @since 0.1
*
* @param string $parserId
*
- * @return string|null
+ * @return string|callable|null
*/
- public function getParserClass( $parserId ) {
+ public function getParserBuilder( $parserId ) {
if ( array_key_exists( $parserId, $this->parsers ) ) {
- return is_string( $this->parsers[$parserId] ) ?
$this->parsers[$parserId] : get_class( $this->parsers[$parserId] );
+ return $this->parsers[$parserId];
}
return null;
@@ -81,16 +88,39 @@
* @param string $parserId
* @param ParserOptions $parserOptions
*
- * @return ValueParser|null
+ * @throws OutOfBoundsException If no parser was registered for
$parserId
+ * @return ValueParser
*/
public function newParser( $parserId, ParserOptions $parserOptions ) {
if ( !array_key_exists( $parserId, $this->parsers ) ) {
- return null;
+ throw new OutOfBoundsException( "No builder registered
for parser ID $parserId" );
}
- $parser = new $this->parsers[$parserId]( $parserOptions );
+ $builder = $this->parsers[$parserId];
+ $parser = $this->instantiateParser( $builder, $parserOptions );
- assert( $parser instanceof ValueParser );
+ return $parser;
+ }
+
+ /**
+ * @param string|callable $builder Either a classname of an
implementation of ValueParser,
+ * or a callable that returns a ValueParser. $options will be
passed to the constructor
+ * or callable, respectively.
+ * @param ParserOptions $options
+ *
+ * @throws LogicException if the builder did not create a ValueParser
+ * @return ValueParser
+ */
+ private function instantiateParser( $builder, ParserOptions $options ) {
+ if ( is_string( $builder ) ) {
+ $parser = new $builder( $options );
+ } else {
+ $parser = call_user_func( $builder, $options );
+ }
+
+ if ( !( $parser instanceof ValueParser ) ) {
+ throw new LogicException( "Invalid parser builder, did
not create an instance of ValueParser." );
+ }
return $parser;
}
diff --git
a/DataValuesCommon/tests/ValueFormatters/ValueFormatterFactoryTest.php
b/DataValuesCommon/tests/ValueFormatters/ValueFormatterFactoryTest.php
index 67e6657..312cecb 100644
--- a/DataValuesCommon/tests/ValueFormatters/ValueFormatterFactoryTest.php
+++ b/DataValuesCommon/tests/ValueFormatters/ValueFormatterFactoryTest.php
@@ -2,21 +2,21 @@
namespace ValueFormatters\Test;
+use ValueFormatters\StringFormatter;
+use ValueFormatters\FormatterOptions;
use ValueFormatters\ValueFormatterFactory;
/**
* @covers ValueFormatters\ValueFormatterFactory
*
- * @file
* @since 0.1
- *
- * @ingroup ValueFormattersTest
*
* @group ValueFormatters
* @group DataValueExtensions
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
+ * @author Daniel Kinzler
*/
class ValueFormatterFactoryTest extends \PHPUnit_Framework_TestCase {
@@ -27,7 +27,10 @@
*/
protected function newFactory() {
return new ValueFormatterFactory( array(
- 'string' => 'ValueFormatters\StringFormatter'
+ 'string' => 'ValueFormatters\StringFormatter',
+ 'dummy' => function( FormatterOptions $options ) {
+ return new StringFormatter( $options );
+ }
) );
}
@@ -45,23 +48,30 @@
public function testGetFormatter() {
$factory = $this->newFactory();
- $options = new \ValueFormatters\FormatterOptions();
+ $options = new FormatterOptions();
foreach ( $factory->getFormatterIds() as $id ) {
- $this->assertInstanceOf(
'ValueFormatters\ValueFormatter', $factory->newFormatter( $id, $options ) );
+ $formatter = $factory->newFormatter( $id, $options );
+ $this->assertInstanceOf(
'ValueFormatters\ValueFormatter', $formatter );
}
- $this->assertInternalType( 'null', $factory->newFormatter( "I'm
in your tests, being rather silly ~=[,,_,,]:3", $options ) );
+ $this->setExpectedException( 'OutOfBoundsException' );
+ $factory->newFormatter( "I'm in your tests, being rather silly
~=[,,_,,]:3", $options );
}
- public function testGetFormatterClass() {
+ public function testGetFormatterBuilder() {
$factory = $this->newFactory();
foreach ( $factory->getFormatterIds() as $id ) {
- $this->assertInternalType( 'string',
$factory->getFormatterClass( $id ) );
+ $builder = $factory->getFormatterBuilder( $id );
+
+ if ( !is_callable( $builder ) ) {
+ $this->assertInternalType( 'string', $builder );
+ }
}
- $this->assertInternalType( 'null', $factory->getFormatterClass(
"I'm in your tests, being rather silly ~=[,,_,,]:3" ) );
+ $builder = $factory->getFormatterBuilder( "I'm in your tests,
being rather silly ~=[,,_,,]:3" );
+ $this->assertInternalType( 'null', $builder );
}
}
diff --git a/DataValuesCommon/tests/ValueParsers/ValueParserFactoryTest.php
b/DataValuesCommon/tests/ValueParsers/ValueParserFactoryTest.php
index ce8cad6..83412d8 100644
--- a/DataValuesCommon/tests/ValueParsers/ValueParserFactoryTest.php
+++ b/DataValuesCommon/tests/ValueParsers/ValueParserFactoryTest.php
@@ -2,21 +2,21 @@
namespace ValueParsers\Test;
+use ValueParsers\NullParser;
+use ValueParsers\ParserOptions;
use ValueParsers\ValueParserFactory;
/**
* @covers ValueParsers\ValueParserFactory
*
- * @file
* @since 0.1
- *
- * @ingroup ValueParsersTest
*
* @group ValueParsers
* @group DataValueExtensions
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroended...@gmail.com >
+ * @author Daniel Kinzler
*/
class ValueParserFactoryTest extends \PHPUnit_Framework_TestCase {
@@ -27,7 +27,10 @@
*/
protected function newFactory() {
return new ValueParserFactory( array(
- 'null' => 'ValueParsers\NullParser'
+ 'null' => 'ValueParsers\NullParser',
+ 'dummy' => function( ParserOptions $options ) {
+ return new NullParser( $options );
+ }
) );
}
@@ -45,29 +48,30 @@
public function testGetParser() {
$factory = $this->newFactory();
- $options = new \ValueParsers\ParserOptions();
+ $options = new ParserOptions();
foreach ( $factory->getParserIds() as $id ) {
- try {
- $parser = $factory->newParser( $id, $options );
- $this->assertInstanceOf(
'ValueParsers\ValueParser', $parser );
- }
- catch ( \Exception $ex ) {
- $this->assertTrue( true, 'Exceptions can be
raised due to not providing required options' );
- }
+ $parser = $factory->newParser( $id, $options );
+ $this->assertInstanceOf( 'ValueParsers\ValueParser',
$parser );
}
- $this->assertInternalType( 'null', $factory->newParser( "I'm in
your tests, being rather silly ~=[,,_,,]:3", $options ) );
+ $this->setExpectedException( 'OutOfBoundsException' );
+ $factory->newParser( "I'm in your tests, being rather silly
~=[,,_,,]:3", $options );
}
- public function testGetParserClass() {
+ public function testGetParserBuilder() {
$factory = $this->newFactory();
foreach ( $factory->getParserIds() as $id ) {
- $this->assertInternalType( 'string',
$factory->getParserClass( $id ) );
+ $builder = $factory->getParserBuilder( $id );
+
+ if ( !is_callable( $builder ) ) {
+ $this->assertInternalType( 'string', $builder );
+ }
}
- $this->assertInternalType( 'null', $factory->getParserClass(
"I'm in your tests, being rather silly ~=[,,_,,]:3" ) );
+ $builder = $factory->getParserBuilder( "I'm in your tests,
being rather silly ~=[,,_,,]:3" );
+ $this->assertInternalType( 'null', $builder );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/98087
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I31b8786f536ec732d53ca33f0859cc6163c7e779
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/DataValues
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
