John Erling Blad has uploaded a new change for review.
https://gerrit.wikimedia.org/r/50770
Change subject: (Bug 44762) Change ml constraints
......................................................................
(Bug 44762) Change ml constraints
Changed from a buggy detection of ML constraints to a simpler one
that is only checking length on save.
It could be that something smarter must be introduced later on
because there are some overly long strings in the database.
Change-Id: I9709521049d7a67e6c760d2ea702c16b76ae002d
---
M repo/Wikibase.hooks.php
M repo/Wikibase.i18n.php
M repo/Wikibase.php
M repo/config/Wikibase.default.php
M repo/includes/EditEntity.php
D repo/includes/MultiLangConstraintDetector.php
M repo/includes/api/ApiModifyLangAttribute.php
M repo/includes/api/ApiSetAliases.php
D repo/tests/phpunit/includes/MultiLangConstraintDetectorTest.php
9 files changed, 19 insertions(+), 367 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/70/50770/1
diff --git a/repo/Wikibase.hooks.php b/repo/Wikibase.hooks.php
index c1c7b88..6ecd88b 100644
--- a/repo/Wikibase.hooks.php
+++ b/repo/Wikibase.hooks.php
@@ -155,7 +155,6 @@
'ItemView',
'LabelDescriptionDuplicateDetector',
'LazyDBConnectionProvider',
- 'MultiLangConstraintDetector',
'NamespaceUtils',
'actions/EditEntityAction',
diff --git a/repo/Wikibase.i18n.php b/repo/Wikibase.i18n.php
index 03e756a..c4c4e43 100644
--- a/repo/Wikibase.i18n.php
+++ b/repo/Wikibase.i18n.php
@@ -96,10 +96,7 @@
'wikibase-move-error' => 'You cannot move pages that are in the data
namespace, and you cannot move pages into it.',
- 'wikibase-warning-constraint-violation-length' => 'A length constraint
is triggered for language code "$1".',
- 'wikibase-error-constraint-violation-label' => 'There is {{PLURAL:$1|a
constraint|constraints}} violation for {{PLURAL:$1|label|labels}} "$3" for
{{PLURAL:$1|language code|language codes}} "$2".',
- 'wikibase-error-constraint-violation-description' => 'There is
{{PLURAL:$1|a constraint|constraints}} violation for
{{PLURAL:$1|description|descriptions}} "$3" for {{PLURAL:$1|language
code|language codes}} "$2".',
- 'wikibase-error-constraint-violation-aliases' => 'There is
{{PLURAL:$1|a constraint|constraints}} violation for
{{PLURAL:$1|alias|aliases}} "$3" for {{PLURAL:$1|language code|language codes}}
"$2".',
+ 'wikibase-constraint-violation' => 'A limit constraint is triggered for
language code "$1.',
'wikibase-error-sitelink-already-used' => 'Site link [[$1:$2]] already
used by item [[$3]].',
'wikibase-error-label-not-unique-wikibase-property' => 'Another
property ($3) already has label "$1" associated with language code $2',
@@ -447,16 +444,8 @@
'wikibase-restrictionedit-tooltip-message' => 'When editing a page (a
data item) is restricted for the current user, this message is displayed in a
tooltip bubble when hovering a tooltip anchor next to an input element, an edit
button or any other button (add, save, remove) that might trigger an action
altering the data item.',
'wikibase-blockeduser-tooltip-message' => 'When the current user is
blocked from editing a page (a data item), this message is displayed in a
tooltip bubble when hovering a tooltip anchor next to an input element, an edit
button or any other button (add, save, remove) that might trigger an action
altering the data item or creating a new item.',
'wikibase-move-error' => 'The data namespace is blocked against moving
pages within it and moving pages into it, because that will make the content
inconsistent both within it and on external wikis. The message uses "pages" in
this case as name for whats moved, but within the data namespace usually
contains "items". See also Wikidatas glossary on
[[m:Wikidata/Glossary#page|page]] and [[m:Wikidata/Glossary#item|item]].',
- 'wikibase-warning-constraint-violation-length' => 'A warning message
that a length constraint is triggered. This will usually come together with a
fatal error message.
+ 'wikibase-constraint-violation' => 'A warning message that a length
constraint is triggered. This will usually come together with a fatal error
message.
* $1 is the language code',
- 'wikibase-error-constraint-violation-label' => 'Error message shown
when a user tries to save a multilabel label that has some constraint violation.
-* $1 is the count of violating languages
-* $2 is the violating languages
-* $3 is the violating string, but this is usually not very useful as the
message is usually given in an edit window',
- 'wikibase-error-constraint-violation-description' => 'Error message
shown when a user tries to save a multilabel description that has some
constraint violation.
-* $1 is the count of violating languages
-* $2 is the violating languages
-* $3 is the violating string, but this is usually not very usefull as the
message is usually given in an edit window',
'wikibase-error-sitelink-already-used' => "Error message shown when an
item can't be saved because it contains a site link already used by another
item. Parameters:
* $1 is the site id (interwiki prefix);
* $2 is the title on the remote site;
diff --git a/repo/Wikibase.php b/repo/Wikibase.php
index f578609..caea17f 100644
--- a/repo/Wikibase.php
+++ b/repo/Wikibase.php
@@ -105,7 +105,6 @@
$wgAutoloadClasses['Wikibase\ItemView'] = $dir
. 'includes/ItemView.php';
$wgAutoloadClasses['Wikibase\LabelDescriptionDuplicateDetector'] = $dir .
'includes/LabelDescriptionDuplicateDetector.php';
$wgAutoloadClasses['Wikibase\Repo\LazyDBConnectionProvider'] = $dir .
'includes/LazyDBConnectionProvider.php';
-$wgAutoloadClasses['Wikibase\MultiLangConstraintDetector'] = $dir .
'includes/MultiLangConstraintDetector.php';
$wgAutoloadClasses['Wikibase\NamespaceUtils'] = $dir .
'includes/NamespaceUtils.php';
$wgAutoloadClasses['Wikibase\PropertyView'] = $dir
. 'includes/PropertyView.php';
diff --git a/repo/config/Wikibase.default.php b/repo/config/Wikibase.default.php
index 5fbac75..b6d6206 100644
--- a/repo/config/Wikibase.default.php
+++ b/repo/config/Wikibase.default.php
@@ -71,5 +71,6 @@
$wgWBRepoSettings['multilang-limits'] = array(
'length' => 250,
);
+$wgWBRepoSettings['multilang-limit'] = 250;
$wgWBRepoSettings['multilang-truncate-length'] = 32;
diff --git a/repo/includes/EditEntity.php b/repo/includes/EditEntity.php
index e8965ff..61b5e60 100644
--- a/repo/includes/EditEntity.php
+++ b/repo/includes/EditEntity.php
@@ -689,16 +689,7 @@
//XXX: havn't we calculated this diff already?
$itemDiff = $entity->getDiff(
$this->getBaseContent()->getEntity() );
}
- //XXX: ...else diff against an empty item?...
-
- $multilangViolationDetector = new MultiLangConstraintDetector();
- $multilangViolationDetector->addConstraintChecks(
- $entity,
- $this->status,
- $itemDiff
- );
-
- if ( !$this->status->isOk() ) {
+ else {
return $this->status;
}
@@ -719,8 +710,8 @@
$entity,
$this->status,
StoreFactory::getStore()->newTermCache(),
- $itemDiff === null ? null :
$itemDiff->getLabelsDiff(),
- $itemDiff === null ? null :
$itemDiff->getDescriptionsDiff()
+ $itemDiff->getLabelsDiff(),
+ $itemDiff->getDescriptionsDiff()
);
}
diff --git a/repo/includes/MultiLangConstraintDetector.php
b/repo/includes/MultiLangConstraintDetector.php
deleted file mode 100644
index d338615..0000000
--- a/repo/includes/MultiLangConstraintDetector.php
+++ /dev/null
@@ -1,193 +0,0 @@
-<?php
-
-namespace Wikibase;
-use Status;
-use Diff\Diff;
-
-/**
- * Detector for multilang constraint violations.
- *
- * 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
- *
- * @since 0.4
- *
- * @file
- * @ingroup WikibaseRepo
- *
- * @licence GNU GPL v2+
- * @author John Erling Blad < [email protected] >
- */
-class MultiLangConstraintDetector {
-
- /**
- * Looks for multilang length violations in the provided entries. If
there
- * is no such conflict, an empty array is returned. If there is to long
entries,
- * an array with multilang strings is returned.
- *
- * @since 0.4
- *
- * @param Entity $entity
- *
- * @return Term[]
- */
- public function getLengthConstraintViolations( array $entries, $limit,
\Status $status ) {
- $foundEntries = array();
-
- foreach ( $entries as $langCode => $langValue ) {
- $toLong = false;
- if ( is_string( $langValue ) ) {
- $toLong = strlen( $langValue ) > $limit;
- }
- elseif ( is_array( $langValue ) ) {
- array_map(
- function( $entry ) use ( &$toLong,
$limit ) {
- $toLong |= is_string( $entry )
&& ( mb_strlen( $entry ) > $limit );
- },
- $langValue
- );
- }
- if ( $toLong ) {
- $foundEntries[$langCode] = $langValue;
- $status->warning(
'wikibase-warning-constraint-violation-length', $langCode );
- }
- }
-
- return $foundEntries;
- }
-
- /**
- * Looks for multilang constraint violations in the provided Entity.
- * If there is a constraint affected by the provided multilang diffs, a
fatal error
- * will be added to the provided status.
- *
- * This could be split out in individual calls, but then the mess show
up in
- * EditEntity and that class should not know to much of the internals.
- *
- * @since 0.4
- *
- * @param Entity $entity The Entity for which to check if there is any
conflict
- * @param Status $status The status to which to add an error if there
is a violation
- * @param Diff|null $diff
- */
- public function addConstraintChecks( Entity $entity, Status $status,
Diff $diff = null, array $limits = null ) {
- global $wgLang;
-
- $truncateLength = Settings::get( 'multilang-truncate-length' );
-
- if ( !isset( $limits ) ) {
- $limits = Settings::get( 'multilang-limits' );
- }
-
- $diffs = array(
- 'label' => $diff === null ? null :
$diff->getLabelsDiff(),
- 'description' => $diff === null ? null :
$diff->getDescriptionsDiff(),
- 'aliases' => $diff === null ? null :
$diff->getAliasesDiff()
- );
-
- $foundSets = array();
-
- if ( wfRunHooks( 'WikibaseAddConstraintChecksForLabel',
- array( &$foundSets['label'], $entity->getLabels(),
$limits, $status ) ) ) {
-
- // default constraints in addition to the ones checked
inside the hook
- $foundSets['label'] = array(
- $this->getLengthConstraintViolations(
$entity->getLabels(), $limits['length'], $status )
- );
- }
-
- if ( wfRunHooks( 'WikibaseAddConstraintChecksForDescription',
- array( &$foundSets['description'],
$entity->getDescriptions(), $limits, $status ) ) ) {
-
- // default constraints in addition to the ones checked
inside the hook
- $foundSets['description'] = array(
- $this->getLengthConstraintViolations(
$entity->getDescriptions(), $limits['length'], $status )
- );
- }
-
- if ( wfRunHooks( 'WikibaseAddConstraintChecksForAliases',
- array( &$foundSets['aliases'],
$entity->getAllAliases(), $limits, $status ) ) ) {
-
- // default constraints in addition to the ones checked
inside the hook
- $foundSets['aliases'] = array(
- $this->getLengthConstraintViolations(
$entity->getAllAliases(), $limits['length'], $status )
- );
- }
-
- foreach ( $foundSets as $section => $set ) {
- $failedLang = array();
- foreach ( $set as $key => $entry ) {
- if ( !empty( $entry ) ) {
- foreach ( $entry as $langCode =>
$langValue) {
- if ( $diffs[$section] === null
|| $this->languageAffectedByDiff( $langCode, $diffs[$section] ) ) {
- $failedLang[$langCode]
= $langValue;
- }
- }
- }
- }
- if ( !empty( $failedLang ) ) {
- // At this point it should be possible to
remove messages for other languages,
- // but unfortunatly there is no method to
remove registered but outdated warnings.
- // We add a tatal error message before we leave.
- $langCodes = array_keys( $failedLang );
- $langCodes = $wgLang->semicolonList( $langCodes
);
- $langValues = array_values( $failedLang );
- $res = array_walk_recursive(
- $langValues,
- function ( &$value ) use (
$truncateLength ) {
- global $wgLang;
- $value = is_string( $value ) ?
$wgLang->truncate( $value, $truncateLength ) : $value;
- }
- );
- $res = array_walk(
- $langValues,
- function ( &$value ) use (
$truncateLength ) {
- global $wgLang;
- $value = is_array( $value ) ?
$wgLang->commaList( $value ) : $value;
- }
- );
- $langValues = $wgLang->semicolonList(
$langValues );
- $status->fatal(
- 'wikibase-error-constraint-violation-'
. $section,
- count($langCodes),
- $langCodes,
- $langValues
- );
- return;
- }
- }
- }
-
- /**
- * Returns if either of the provided multilang diffs affect a certain
language.
- *
- * @since 0.4
- *
- * @param string $languageCode
- * @param Diff|null $diff
- *
- * @return boolean
- */
- protected function languageAffectedByDiff( $languageCode, Diff $diff =
null ) {
- $c = $diff->getOperations();
-
- if ( $diff !== null && array_key_exists( $languageCode, $c ) ) {
- return true;
- }
-
- return false;
- }
-
-}
\ No newline at end of file
diff --git a/repo/includes/api/ApiModifyLangAttribute.php
b/repo/includes/api/ApiModifyLangAttribute.php
index 3dbd139..d3d8f9b 100644
--- a/repo/includes/api/ApiModifyLangAttribute.php
+++ b/repo/includes/api/ApiModifyLangAttribute.php
@@ -25,9 +25,9 @@
protected function validateParameters( array $params ) {
parent::validateParameters( $params );
- // Note that language should always exist as a prerequisite for
this
- // call to succeede. The param value will not always exist
because
- // that signals a label to remove.
+ if ( isset( $params['language'] ) && mb_strlen(
$params['value'] ) > Settings::get( 'multilang-limit' ) ) {
+ $this->dieUsage( $this->msg(
'wikibase-constraint-violation', $params['language'] )->text(),
'constraint-violation' );
+ }
}
/**
diff --git a/repo/includes/api/ApiSetAliases.php
b/repo/includes/api/ApiSetAliases.php
index 92a0161..76b3420 100644
--- a/repo/includes/api/ApiSetAliases.php
+++ b/repo/includes/api/ApiSetAliases.php
@@ -46,6 +46,16 @@
if ( !( ( isset( $params['add'] ) || isset( $params['remove'] )
) XOR isset( $params['set'] ) ) ) {
$this->dieUsage( $this->msg(
'wikibase-api-aliases-invalid-list' )->text(), 'aliases-invalid-list' );
}
+ foreach ( array( 'add', 'set' ) as $operation ) {
+ if ( !isset( $params['add'] ) ) {
+ continue;
+ }
+ foreach ( $params[$operation] as $alias ) {
+ if ( mb_strlen( $alias ) > Settings::get(
'multilang-limit' ) ) {
+ $this->dieUsage( $this->msg(
'wikibase-constraint-violation', $params['language'] )->text(),
'constraint-violation' );
+ }
+ }
+ }
}
/**
diff --git a/repo/tests/phpunit/includes/MultiLangConstraintDetectorTest.php
b/repo/tests/phpunit/includes/MultiLangConstraintDetectorTest.php
deleted file mode 100644
index 4656974..0000000
--- a/repo/tests/phpunit/includes/MultiLangConstraintDetectorTest.php
+++ /dev/null
@@ -1,144 +0,0 @@
-<?php
-
-/**
- * Tests Wikibase\MultiLangConstraintDetector.
- *
- * 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 0.4
- *
- * @ingroup WikibaseRepoTest
- * @ingroup Test
- *
- * @group Wikibase
- * @group MultiLangConstraintDetector
- *
- * @licence GNU GPL v2+
- * @author John Erling Blad < [email protected] >
- */
-class MultiLangConstraintDetectorTest extends \MediaWikiTestCase {
-
- static $limit = 23;
- static $short = 'This is a short string'; // 22 bytes
- static $long = 'This is a to long string'; // 24 bytes
-
- public function mlStringProvider() {
- return array(
- array(
- array( 'en' => self::$short ), // 22 bytes
- array(),
- false
- ),
- array(
- array( 'en' => self::$long ), // 24 bytes
- array( 'en' => self::$long ), // 24 bytes
- true
- )
- );
- }
-
- /**
- * @dataProvider mlStringProvider
- */
- public function testGetStringLengthConstraintViolations( $data,
$expected, $fatal ) {
- $status = Status::newGood();
- $extData = array(
- 'da' => self::$short,
- 'de' => self::$long,
- );
- $extExpected = array(
- 'de' => self::$long,
- );
- $detector = new Wikibase\MultiLangConstraintDetector();
- $result = $detector->getLengthConstraintViolations(
array_merge( $extData, $data ), self::$limit, $status );
- $this->assertEquals( array_merge( $extExpected, $expected ),
$result );
- $this->assertEquals( empty( $result ), $status->isGood() );
- }
-
- /**
- * @dataProvider mlStringProvider
- */
- public function testGetArrayLengthConstraintViolations( $data,
$expected, $fatal ) {
- $status = Status::newGood();
- $data = array_map( function($v) { return array($v); }, $data );
- $expected = array_map( function($v) { return array($v); },
$expected );
- $extData = array(
- 'da' => array( self::$short ),
- 'de' => array( self::$long ),
- );
- $extExpected = array(
- 'de' => array( self::$long ),
- );
- $detector = new Wikibase\MultiLangConstraintDetector();
- $result = $detector->getLengthConstraintViolations(
array_merge_recursive( $extData, $data ), self::$limit, $status );
- $this->assertEquals( array_merge_recursive( $extExpected,
$expected ), $result );
- $this->assertEquals( empty( $result ), $status->isGood() );
- }
-
- /**
- * @dataProvider mlStringProvider
- */
- public function testAddStringConstraintChecks( $data, $expected, $fatal
) {
- $status = Status::newGood();
- $detector = new Wikibase\MultiLangConstraintDetector();
-
- $extData = array(
- 'da' => self::$short,
- 'de' => self::$long,
- );
-
- // test labels ---------------
- $baseEntity = new Wikibase\Item( array( 'label' => $extData ) );
- $newEntity = new Wikibase\Item( array( 'label' => array_merge(
$extData, $data ) ) );
- $diff = $baseEntity->getDiff( $newEntity );
- $detector->addConstraintChecks( $newEntity, $status, $diff,
array( 'length' => self::$limit ) );
-
- $this->assertEquals( $fatal, !$status->isOk() );
-
- // test descriptions ---------------
- $baseEntity = new Wikibase\Item( array( 'description' =>
$extData ) );
- $newEntity = new Wikibase\Item( array( 'description' =>
array_merge( $extData, $data ) ) );
- $diff = $baseEntity->getDiff( $newEntity );
- $detector->addConstraintChecks( $newEntity, $status, $diff,
array( 'length' => self::$limit ) );
-
- $this->assertEquals( $fatal, !$status->isOk() );
- }
-
- /**
- * @dataProvider mlStringProvider
- */
- public function testAddArrayConstraintChecks( $data, $expected, $fatal
) {
- $status = Status::newGood();
- $detector = new Wikibase\MultiLangConstraintDetector();
-
- $data = array_map( function($v) { return array($v); }, $data );
-
- $extData = array(
- 'da' => array( self::$short ),
- 'de' => array( self::$long ),
- );
-
- // test aliases ---------------
- $baseEntity = new Wikibase\Item( array( 'aliases' => $extData )
);
- $newEntity = new Wikibase\Item( array( 'aliases' =>
array_merge_recursive( $extData, $data ) ) );
- $diff = $baseEntity->getDiff( $newEntity );
- $detector->addConstraintChecks( $newEntity, $status, $diff,
array( 'length' => self::$limit ) );
-
- $this->assertEquals( $fatal, !$status->isOk() );
-
- }
-}
\ No newline at end of file
--
To view, visit https://gerrit.wikimedia.org/r/50770
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9709521049d7a67e6c760d2ea702c16b76ae002d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: John Erling Blad <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
