Mailing List Archive

[MediaWiki-commits] [Gerrit] mediawiki...WikibaseQualityConstraints[master]: Rewrite ValueCountCheckerHelper
Lucas Werkmeister (WMDE) has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/405033 )

Change subject: Rewrite ValueCountCheckerHelper
......................................................................

Rewrite ValueCountCheckerHelper

ValueCountCheckerHelper now receives a Snak[] instead of a
StatementList, and also receives the property ID to look for instead of
returning an array of the counts of all properties. SingleValueChecker
and MultiValueChecker are adjusted accordingly and pass the sibling
snaks of the context into ValueCountCheckerHelper.

This does not yet provide proper support for checking those constraints
on qualifiers and references, since the checkers declare to not support
those types yet. getSupportedContextTypes will be adjusted in a separate
commit which will also add tests for checking these constraints on
qualifiers and references.

Bug: T175566
Change-Id: I301400edd29b0a10ffc131fd5a20bf6e3b193852
---
M src/ConstraintCheck/Checker/MultiValueChecker.php
M src/ConstraintCheck/Checker/SingleValueChecker.php
M src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
M tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
4 files changed, 44 insertions(+), 50 deletions(-)


git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseQualityConstraints refs/changes/33/405033/1

diff --git a/src/ConstraintCheck/Checker/MultiValueChecker.php b/src/ConstraintCheck/Checker/MultiValueChecker.php
index 6f200d4..7014060 100644
--- a/src/ConstraintCheck/Checker/MultiValueChecker.php
+++ b/src/ConstraintCheck/Checker/MultiValueChecker.php
@@ -64,9 +64,12 @@

$parameters = [];

- $propertyCountArray = $this->valueCountCheckerHelper->getPropertyCount( $context->getEntity()->getStatements() );
+ $propertyCount = $this->valueCountCheckerHelper->getPropertyCount(
+ $context->getSiblingSnaks(),
+ $propertyId
+ ) + 1; // the sibling snaks do not include the context’s own snak

- if ( $propertyCountArray[$propertyId->getSerialization()] <= 1 ) {
+ if ( $propertyCount <= 1 ) {
$message = wfMessage( "wbqc-violation-message-multi-value" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
} else {
diff --git a/src/ConstraintCheck/Checker/SingleValueChecker.php b/src/ConstraintCheck/Checker/SingleValueChecker.php
index 5218b78..dcf26fc 100644
--- a/src/ConstraintCheck/Checker/SingleValueChecker.php
+++ b/src/ConstraintCheck/Checker/SingleValueChecker.php
@@ -64,9 +64,12 @@

$parameters = [];

- $propertyCountArray = $this->valueCountCheckerHelper->getPropertyCount( $context->getEntity()->getStatements() );
+ $propertyCount = $this->valueCountCheckerHelper->getPropertyCount(
+ $context->getSiblingSnaks(),
+ $propertyId
+ ) + 1; // the sibling snaks do not include the context’s own snak

- if ( $propertyCountArray[$propertyId->getSerialization()] > 1 ) {
+ if ( $propertyCount > 1 ) {
$message = wfMessage( "wbqc-violation-message-single-value" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
} else {
diff --git a/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php b/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
index 849e770..f9b675c 100644
--- a/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
+++ b/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
@@ -2,43 +2,31 @@

namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Helper;

-use Wikibase\DataModel\Statement\Statement;
-use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Snak\Snak;

/**
* Class for helper functions for value count checkers.
*
- * @author BP2014N1
+ * @author Lucas Werkmeister
* @license GNU GPL v2+
*/
class ValueCountCheckerHelper {

/**
- * @var int[]
- */
- private $propertyCount;
-
- /**
- * @param StatementList $statements
+ * Count the number of snaks with the given property ID.
*
- * @return int[]
+ * @param Snak[] $snaks
+ * @param PropertyId $propertyId
+ * @return int
*/
- public function getPropertyCount( StatementList $statements ) {
- if ( !isset( $this->propertyCount ) ) {
- $this->propertyCount = [];
-
- /** @var Statement $statement */
- foreach ( $statements as $statement ) {
- $counter = $statement->getRank() === Statement::RANK_DEPRECATED ? 0 : 1;
- if ( array_key_exists( $statement->getPropertyId()->getSerialization(), $this->propertyCount ) ) {
- $this->propertyCount[$statement->getPropertyId()->getSerialization()] += $counter;
- } else {
- $this->propertyCount[$statement->getPropertyId()->getSerialization()] = $counter;
- }
+ public function getPropertyCount( array $snaks, PropertyId $propertyId ) {
+ return count( array_filter(
+ $snaks,
+ function ( Snak $snak ) use ( $propertyId ) {
+ return $snak->getPropertyId()->equals( $propertyId );
}
- }
-
- return $this->propertyCount;
+ ) );
}

}
diff --git a/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php b/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
index 9d75a4e..da5d3a9 100644
--- a/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
+++ b/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
@@ -3,12 +3,9 @@
namespace WikibaseQuality\ConstraintReport\Test\ValueCountChecker;

use PHPUnit_Framework_TestCase;
-use Wikibase\DataModel\Statement\Statement;
-use Wikibase\DataModel\Snak\PropertyValueSnak;
-use Wikibase\DataModel\Statement\StatementList;
use Wikibase\DataModel\Entity\PropertyId;
-use Wikibase\DataModel\Entity\EntityIdValue;
-use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Snak\PropertyNoValueSnak;
+use Wikibase\DataModel\Snak\PropertySomeValueSnak;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ValueCountCheckerHelper;

/**
@@ -16,32 +13,35 @@
*
* @group WikibaseQualityConstraints
*
- * @author BP2014N1
+ * @author Lucas Werkmeister
* @license GNU GPL v2+
*/
class ValueCountCheckerHelperTest extends PHPUnit_Framework_TestCase {

/**
- * @var StatementList
+ * @dataProvider getPropertyCountProvider
*/
- private $statementList;
+ public function testGetPropertyCount( $snaks, $propertyIdSerialization, $expectedCount ) {
+ $propertyId = new PropertyId( $propertyIdSerialization );
+ $helper = new ValueCountCheckerHelper();
+ $propertyCount = $helper->getPropertyCount( $snaks, $propertyId );

- protected function setUp() {
- parent::setUp();
- $statement1 = new Statement( new PropertyValueSnak( new PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q1' ) ) ) );
- $statement2 = new Statement( new PropertyValueSnak( new PropertyId( 'P2' ), new EntityIdValue( new ItemId( 'Q2' ) ) ) );
- $statement3 = new Statement( new PropertyValueSnak( new PropertyId( 'P2' ), new EntityIdValue( new ItemId( 'Q3' ) ) ) );
- $statement4 = new Statement( new PropertyValueSnak( new PropertyId( 'P2' ), new EntityIdValue( new ItemId( 'Q4' ) ) ) );
- $statement4->setRank( Statement::RANK_DEPRECATED );
- $this->statementList = new StatementList( [ $statement1, $statement2, $statement3, $statement4 ] );
+ $this->assertSame( $expectedCount, $propertyCount );
}

- public function testGetPropertyCount() {
- $checker = new ValueCountCheckerHelper();
- $propertyCount = $checker->getPropertyCount( $this->statementList );
+ public function getPropertyCountProvider() {
+ $p1_1 = new PropertyNoValueSnak( new PropertyId( 'P1' ) );
+ $p1_2 = new PropertySomeValueSnak( new PropertyId( 'P1' ) );
+ $p2 = new PropertyNoValueSnak( new PropertyId( 'P2' ) );

- $this->assertEquals( 1, $propertyCount['P1'] );
- $this->assertEquals( 2, $propertyCount['P2'] );
+ return [
+ 'empty' => [ [], 'P1', 0 ],
+ 'only other property' => [ [ $p2 ], 'P1', 0 ],
+ 'only searched property' => [ [ $p1_1 ], 'P1', 1 ],
+ 'both properties' => [ [ $p1_1, $p2 ], 'P1', 1 ],
+ 'searched property multiple times' => [ [ $p1_1, $p1_2 ], 'P1', 2 ],
+ 'same snak multiple times' => [ [ $p1_1, $p1_1, $p1_2 ], 'P1', 3 ],
+ ];
}

}

--
To view, visit https://gerrit.wikimedia.org/r/405033
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I301400edd29b0a10ffc131fd5a20bf6e3b193852
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <lucas.werkmeister@wikimedia.de>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
[MediaWiki-commits] [Gerrit] mediawiki...WikibaseQualityConstraints[master]: Rewrite ValueCountCheckerHelper [ In reply to ]
jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/405033 )

Change subject: Rewrite ValueCountCheckerHelper
......................................................................


Rewrite ValueCountCheckerHelper

ValueCountCheckerHelper now receives a Snak[] instead of a
StatementList, and also receives the property ID to look for instead of
returning an array of the counts of all properties. SingleValueChecker
and MultiValueChecker are adjusted accordingly and pass the snak group
of the context into ValueCountCheckerHelper.

This does not yet provide proper support for checking those constraints
on qualifiers and references, since the checkers declare to not support
those types yet. getSupportedContextTypes will be adjusted in a separate
commit which will also add tests for checking these constraints on
qualifiers and references.

Bug: T175566
Change-Id: I301400edd29b0a10ffc131fd5a20bf6e3b193852
---
M src/ConstraintCheck/Checker/MultiValueChecker.php
M src/ConstraintCheck/Checker/SingleValueChecker.php
M src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
M tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
4 files changed, 44 insertions(+), 50 deletions(-)

Approvals:
jenkins-bot: Verified
Thiemo Kreuz (WMDE): Looks good to me, approved



diff --git a/src/ConstraintCheck/Checker/MultiValueChecker.php b/src/ConstraintCheck/Checker/MultiValueChecker.php
index 6f200d4..33b6829 100644
--- a/src/ConstraintCheck/Checker/MultiValueChecker.php
+++ b/src/ConstraintCheck/Checker/MultiValueChecker.php
@@ -64,9 +64,12 @@

$parameters = [];

- $propertyCountArray = $this->valueCountCheckerHelper->getPropertyCount( $context->getEntity()->getStatements() );
+ $propertyCount = $this->valueCountCheckerHelper->getPropertyCount(
+ $context->getSnakGroup(),
+ $propertyId
+ );

- if ( $propertyCountArray[$propertyId->getSerialization()] <= 1 ) {
+ if ( $propertyCount <= 1 ) {
$message = wfMessage( "wbqc-violation-message-multi-value" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
} else {
diff --git a/src/ConstraintCheck/Checker/SingleValueChecker.php b/src/ConstraintCheck/Checker/SingleValueChecker.php
index 5218b78..fe277ff 100644
--- a/src/ConstraintCheck/Checker/SingleValueChecker.php
+++ b/src/ConstraintCheck/Checker/SingleValueChecker.php
@@ -64,9 +64,12 @@

$parameters = [];

- $propertyCountArray = $this->valueCountCheckerHelper->getPropertyCount( $context->getEntity()->getStatements() );
+ $propertyCount = $this->valueCountCheckerHelper->getPropertyCount(
+ $context->getSnakGroup(),
+ $propertyId
+ );

- if ( $propertyCountArray[$propertyId->getSerialization()] > 1 ) {
+ if ( $propertyCount > 1 ) {
$message = wfMessage( "wbqc-violation-message-single-value" )->escaped();
$status = CheckResult::STATUS_VIOLATION;
} else {
diff --git a/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php b/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
index 849e770..f9b675c 100644
--- a/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
+++ b/src/ConstraintCheck/Helper/ValueCountCheckerHelper.php
@@ -2,43 +2,31 @@

namespace WikibaseQuality\ConstraintReport\ConstraintCheck\Helper;

-use Wikibase\DataModel\Statement\Statement;
-use Wikibase\DataModel\Statement\StatementList;
+use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\DataModel\Snak\Snak;

/**
* Class for helper functions for value count checkers.
*
- * @author BP2014N1
+ * @author Lucas Werkmeister
* @license GNU GPL v2+
*/
class ValueCountCheckerHelper {

/**
- * @var int[]
- */
- private $propertyCount;
-
- /**
- * @param StatementList $statements
+ * Count the number of snaks with the given property ID.
*
- * @return int[]
+ * @param Snak[] $snaks
+ * @param PropertyId $propertyId
+ * @return int
*/
- public function getPropertyCount( StatementList $statements ) {
- if ( !isset( $this->propertyCount ) ) {
- $this->propertyCount = [];
-
- /** @var Statement $statement */
- foreach ( $statements as $statement ) {
- $counter = $statement->getRank() === Statement::RANK_DEPRECATED ? 0 : 1;
- if ( array_key_exists( $statement->getPropertyId()->getSerialization(), $this->propertyCount ) ) {
- $this->propertyCount[$statement->getPropertyId()->getSerialization()] += $counter;
- } else {
- $this->propertyCount[$statement->getPropertyId()->getSerialization()] = $counter;
- }
+ public function getPropertyCount( array $snaks, PropertyId $propertyId ) {
+ return count( array_filter(
+ $snaks,
+ function ( Snak $snak ) use ( $propertyId ) {
+ return $snak->getPropertyId()->equals( $propertyId );
}
- }
-
- return $this->propertyCount;
+ ) );
}

}
diff --git a/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php b/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
index 9d75a4e..da5d3a9 100644
--- a/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
+++ b/tests/phpunit/Checker/ValueCountChecker/ValueCountCheckerHelperTest.php
@@ -3,12 +3,9 @@
namespace WikibaseQuality\ConstraintReport\Test\ValueCountChecker;

use PHPUnit_Framework_TestCase;
-use Wikibase\DataModel\Statement\Statement;
-use Wikibase\DataModel\Snak\PropertyValueSnak;
-use Wikibase\DataModel\Statement\StatementList;
use Wikibase\DataModel\Entity\PropertyId;
-use Wikibase\DataModel\Entity\EntityIdValue;
-use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Snak\PropertyNoValueSnak;
+use Wikibase\DataModel\Snak\PropertySomeValueSnak;
use WikibaseQuality\ConstraintReport\ConstraintCheck\Helper\ValueCountCheckerHelper;

/**
@@ -16,32 +13,35 @@
*
* @group WikibaseQualityConstraints
*
- * @author BP2014N1
+ * @author Lucas Werkmeister
* @license GNU GPL v2+
*/
class ValueCountCheckerHelperTest extends PHPUnit_Framework_TestCase {

/**
- * @var StatementList
+ * @dataProvider getPropertyCountProvider
*/
- private $statementList;
+ public function testGetPropertyCount( $snaks, $propertyIdSerialization, $expectedCount ) {
+ $propertyId = new PropertyId( $propertyIdSerialization );
+ $helper = new ValueCountCheckerHelper();
+ $propertyCount = $helper->getPropertyCount( $snaks, $propertyId );

- protected function setUp() {
- parent::setUp();
- $statement1 = new Statement( new PropertyValueSnak( new PropertyId( 'P1' ), new EntityIdValue( new ItemId( 'Q1' ) ) ) );
- $statement2 = new Statement( new PropertyValueSnak( new PropertyId( 'P2' ), new EntityIdValue( new ItemId( 'Q2' ) ) ) );
- $statement3 = new Statement( new PropertyValueSnak( new PropertyId( 'P2' ), new EntityIdValue( new ItemId( 'Q3' ) ) ) );
- $statement4 = new Statement( new PropertyValueSnak( new PropertyId( 'P2' ), new EntityIdValue( new ItemId( 'Q4' ) ) ) );
- $statement4->setRank( Statement::RANK_DEPRECATED );
- $this->statementList = new StatementList( [ $statement1, $statement2, $statement3, $statement4 ] );
+ $this->assertSame( $expectedCount, $propertyCount );
}

- public function testGetPropertyCount() {
- $checker = new ValueCountCheckerHelper();
- $propertyCount = $checker->getPropertyCount( $this->statementList );
+ public function getPropertyCountProvider() {
+ $p1_1 = new PropertyNoValueSnak( new PropertyId( 'P1' ) );
+ $p1_2 = new PropertySomeValueSnak( new PropertyId( 'P1' ) );
+ $p2 = new PropertyNoValueSnak( new PropertyId( 'P2' ) );

- $this->assertEquals( 1, $propertyCount['P1'] );
- $this->assertEquals( 2, $propertyCount['P2'] );
+ return [
+ 'empty' => [ [], 'P1', 0 ],
+ 'only other property' => [ [ $p2 ], 'P1', 0 ],
+ 'only searched property' => [ [ $p1_1 ], 'P1', 1 ],
+ 'both properties' => [ [ $p1_1, $p2 ], 'P1', 1 ],
+ 'searched property multiple times' => [ [ $p1_1, $p1_2 ], 'P1', 2 ],
+ 'same snak multiple times' => [ [ $p1_1, $p1_1, $p1_2 ], 'P1', 3 ],
+ ];
}

}

--
To view, visit https://gerrit.wikimedia.org/r/405033
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I301400edd29b0a10ffc131fd5a20bf6e3b193852
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <lucas.werkmeister@wikimedia.de>
Gerrit-Reviewer: Thiemo Kreuz (WMDE) <thiemo.kreuz@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits