Addshore has uploaded a new change for review.
https://gerrit.wikimedia.org/r/227250
Change subject: Use Assert in ResultBuilder
......................................................................
Use Assert in ResultBuilder
Change-Id: Id78c9077a7d02c5fe0c7fd4f8d63e7c9782451d9
---
M repo/includes/api/ResultBuilder.php
M repo/tests/phpunit/includes/api/ResultBuilderTest.php
2 files changed, 26 insertions(+), 84 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/50/227250/1
diff --git a/repo/includes/api/ResultBuilder.php
b/repo/includes/api/ResultBuilder.php
index 407214b..dbeba62 100644
--- a/repo/includes/api/ResultBuilder.php
+++ b/repo/includes/api/ResultBuilder.php
@@ -3,7 +3,6 @@
namespace Wikibase\Repo\Api;
use ApiResult;
-use InvalidArgumentException;
use Revision;
use SiteStore;
use Status;
@@ -21,6 +20,7 @@
use Wikibase\LanguageFallbackChain;
use Wikibase\Lib\Serializers\SerializationOptions;
use Wikibase\Lib\Store\EntityTitleLookup;
+use Wikimedia\Assert\Assert;
/**
* Builder for Api Results
@@ -85,21 +85,15 @@
* @param SiteStore $siteStore
* @param PropertyDataTypeLookup $dataTypeLookup
* @param bool $isRawMode when special elements such as '_element' are
needed by the formatter.
- *
- * @throws InvalidArgumentException
*/
public function __construct(
- $result,
+ ApiResult $result,
EntityTitleLookup $entityTitleLookup,
SerializerFactory $serializerFactory,
SiteStore $siteStore,
PropertyDataTypeLookup $dataTypeLookup,
$isRawMode
) {
- if ( !$result instanceof ApiResult ) {
- throw new InvalidArgumentException( 'Result builder
must be constructed with an ApiResult' );
- }
-
$this->result = $result;
$this->entityTitleLookup = $entityTitleLookup;
$this->serializerFactory = $serializerFactory;
@@ -114,17 +108,15 @@
* @since 0.5
*
* @param $success bool|int|null
- *
- * @throws InvalidArgumentException
*/
public function markSuccess( $success = true ) {
$value = (int)$success;
- if ( $value !== 1 && $value !== 0 ) {
- throw new InvalidArgumentException(
- '$success must evaluate to either 1 or 0 when
casted to integer'
- );
- }
+ Assert::parameter(
+ $value == 1 || $value == 0,
+ '$success',
+ '$success must evaluate to either 1 or 0 when casted to
integer'
+ );
$this->result->addValue( null, 'success', $value );
}
@@ -146,13 +138,11 @@
* @param $name string
* @param $values array
* @param string $tag tag name to use for elements of $values
- *
- * @throws InvalidArgumentException
*/
public function setList( $path, $name, array $values, $tag ) {
$this->checkPathType( $path );
- $this->checkNameIsString( $name );
- $this->checkTagIsString( $tag );
+ Assert::parameterType( 'string', $name, '$name' );
+ Assert::parameterType( 'string', $tag, '$tag' );
if ( $this->isRawMode ) {
// Unset first, so we don't make the tag name an actual
value.
@@ -180,12 +170,10 @@
* @param $path array|string|null
* @param $name string
* @param $value mixed
- *
- * @throws InvalidArgumentException
*/
public function setValue( $path, $name, $value ) {
$this->checkPathType( $path );
- $this->checkNameIsString( $name );
+ Assert::parameterType( 'string', $name, '$name' );
$this->checkValueIsNotList( $value );
$this->result->addValue( $path, $name, $value );
@@ -210,14 +198,11 @@
* May be ignored even if given, based on $this->result->getIsRawMode().
* @param $value mixed
* @param string $tag tag name to use for $value in indexed mode
- *
- * @throws InvalidArgumentException
*/
public function appendValue( $path, $key, $value, $tag ) {
$this->checkPathType( $path );
$this->checkKeyType( $key );
- $this->checkTagIsString( $tag );
-
+ Assert::parameterType( 'string', $tag, '$tag' );
$this->checkValueIsNotList( $value );
if ( $this->isRawMode ) {
@@ -230,61 +215,35 @@
/**
* @param array|string|null $path
- *
- * @throws InvalidArgumentException
*/
private function checkPathType( $path ) {
- if ( is_string( $path ) ) {
- $path = array( $path );
- }
-
- if ( !is_array( $path ) && $path !== null ) {
- throw new InvalidArgumentException( '$path must be an
array (or null)' );
- }
- }
-
- /**
- * @param string $name
- *
- * @throws InvalidArgumentException
- */
- private function checkNameIsString( $name ) {
- if ( !is_string( $name ) ) {
- throw new InvalidArgumentException( '$name must be a
string' );
- }
+ Assert::parameter(
+ is_string( $path ) || is_array( $path ) || is_null(
$path ),
+ '$path',
+ '$path must be an array (or null)'
+ );
}
/**
* @param $key int|string|null the key to use when appending, or null
for automatic.
- *
- * @throws InvalidArgumentException
*/
private function checkKeyType( $key ) {
- if ( $key !== null && !is_string( $key ) && !is_int( $key ) ) {
- throw new InvalidArgumentException( '$key must be a
string, int, or null' );
- }
- }
-
- /**
- * @param string $tag tag name to use for elements of $values
- *
- * @throws InvalidArgumentException
- */
- private function checkTagIsString( $tag ) {
- if ( !is_string( $tag ) ) {
- throw new InvalidArgumentException( '$tag must be a
string' );
- }
+ Assert::parameter(
+ is_string( $key ) || is_int( $key ) || is_null( $key ),
+ '$key',
+ '$key must be an array (or null)'
+ );
}
/**
* @param mixed $value
- *
- * @throws InvalidArgumentException
*/
private function checkValueIsNotList( $value ) {
- if ( is_array( $value ) && isset( $value[0] ) ) {
- throw new InvalidArgumentException( '$value must not be
a list' );
- }
+ Assert::parameter(
+ !( is_array( $value ) && isset( $value[0] ) ),
+ '$value',
+ '$value must not be a list'
+ );
}
/**
diff --git a/repo/tests/phpunit/includes/api/ResultBuilderTest.php
b/repo/tests/phpunit/includes/api/ResultBuilderTest.php
index 66d9b7f..2c2135d 100644
--- a/repo/tests/phpunit/includes/api/ResultBuilderTest.php
+++ b/repo/tests/phpunit/includes/api/ResultBuilderTest.php
@@ -95,23 +95,6 @@
}
/**
- * @dataProvider provideBadConstructionData
- */
- public function testBadConstruction( $result ) {
- $this->setExpectedException( 'InvalidArgumentException' );
- $this->getResultBuilder( $result );
- }
-
- public function provideBadConstructionData() {
- return array(
- array( null ),
- array( 1234 ),
- array( "imastring" ),
- array( array() ),
- );
- }
-
- /**
* @dataProvider provideMarkResultSuccess
*/
public function testMarkResultSuccess( $param, $expected ) {
--
To view, visit https://gerrit.wikimedia.org/r/227250
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id78c9077a7d02c5fe0c7fd4f8d63e7c9782451d9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: JanZerebecki <jan.wikime...@zerebecki.de>
Gerrit-Reviewer: Jeroen De Dauw <jeroended...@gmail.com>
Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
