Noella94 has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/405222 )
Change subject: [WIP]Replacing target Arrays with Objects.
......................................................................
[WIP]Replacing target Arrays with Objects.
Instead of passing targets as Arrays passing now but target Objects
Bug: T178215
Bug: T178431
Change-Id: I2c7fb5d30fd20bf06393cced73027089f2533b6e
---
M extension.json
M includes/ApiEditMassMessageList.php
M includes/MassMessage.php
M includes/MassMessageTargets.php
A includes/Target.php
M includes/content/MassMessageListContent.php
M includes/content/MassMessageListContentHandler.php
M tests/phpunit/content/MassMessageContentTest.php
8 files changed, 78 insertions(+), 62 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MassMessage
refs/changes/22/405222/1
diff --git a/extension.json b/extension.json
index 6a1b354..5d0ec41 100644
--- a/extension.json
+++ b/extension.json
@@ -225,6 +225,7 @@
"MediaWiki\\MassMessage\\MassMessageListContent":
"includes/content/MassMessageListContent.php",
"MediaWiki\\MassMessage\\MassMessageListContentHandler":
"includes/content/MassMessageListContentHandler.php",
"MediaWiki\\MassMessage\\MassMessageListDiffEngine":
"includes/content/MassMessageListDiffEngine.php",
+ "MediaWiki\\MassMessage\\Target": "includes/Target.php",
"MediaWiki\\MassMessage\\MassMessageTestCase":
"tests/phpunit/MassMessageTestCase.php",
"MediaWiki\\MassMessage\\MassMessageApiTestCase":
"tests/phpunit/MassMessageApiTestCase.php"
},
diff --git a/includes/ApiEditMassMessageList.php
b/includes/ApiEditMassMessageList.php
index 9ffd8e7..f53068e 100644
--- a/includes/ApiEditMassMessageList.php
+++ b/includes/ApiEditMassMessageList.php
@@ -44,14 +44,14 @@
foreach ( $data['add'] as $page ) {
$target =
MassMessageListContentHandler::extractTarget( $page );
- if ( isset( $target['errors'] ) ) {
+ if ( isset( $target->error ) ) {
$item = [ '*' => $page ];
- foreach ( $target['errors'] as $error )
{
+ foreach ( $target->error as $error ) {
$item[$error] = '';
}
$invalidAdd[] = $item;
} else {
- $newTargets[] = $target;
+ $newTargets = $target;
}
}
@@ -66,10 +66,10 @@
foreach ( $data['remove'] as $page ) {
$target =
MassMessageListContentHandler::extractTarget( $page );
- if ( isset( $target['errors'] ) || !in_array(
$target, $newTargets ) ) {
+ if ( isset( $target->error ) || !in_array(
$target, $newTargets ) ) {
$invalidRemove[] = $page;
} else {
- $toRemove[] = $target;
+ $toRemove = $target;
}
}
diff --git a/includes/MassMessage.php b/includes/MassMessage.php
index 8a79902..86c890a 100644
--- a/includes/MassMessage.php
+++ b/includes/MassMessage.php
@@ -58,20 +58,20 @@
return self::parserError( 'massmessage-parse-badpage',
$page );
}
- $data = [ 'title' => $page, 'site' => trim( $site ) ];
- if ( $data['site'] === '' ) {
- $data['site'] = UrlHelper::getBaseUrl(
$wgCanonicalServer );
- $data['wiki'] = wfWikiID();
+ $data = new Target();
+ if ( $data->site === '' ) {
+ $data->site = UrlHelper::getBaseUrl( $wgCanonicalServer
);
+ $data->wiki = wfWikiID();
} else {
- $data['wiki'] = DatabaseLookup::getDBName(
$data['site'] );
- if ( $data['wiki'] === null ) {
+ $data->wiki = DatabaseLookup::getDBName( $data->site );
+ if ( $data->wiki === null ) {
return self::parserError(
'massmessage-parse-badurl', $site );
}
- if ( !$wgAllowGlobalMessaging && $data['wiki'] !==
wfWikiID() ) {
+ if ( !$wgAllowGlobalMessaging && $data->wiki !==
wfWikiID() ) {
return self::parserError(
'massmessage-global-disallowed' );
}
}
- if ( $data['wiki'] === wfWikiID() && $titleObj->isExternal() ) {
+ if ( $data->wiki === wfWikiID() && $titleObj->isExternal() ) {
// interwiki links don't work
if ( $wgAllowGlobalMessaging ) {
// tell them they need to use the |site=
parameter
diff --git a/includes/MassMessageTargets.php b/includes/MassMessageTargets.php
index 9052682..230b6e2 100644
--- a/includes/MassMessageTargets.php
+++ b/includes/MassMessageTargets.php
@@ -22,8 +22,7 @@
* wiki: The ID of the wiki (wfWikiID() for the local wiki)
* site: The hostname and port (if exists) of the wiki
*
- * Normalized targets are briefly cached because it can be expensive to
parse PF targets on both
- * preview and save in SpecialMassMessage.
+ * Normalized targets are briefly cached because it can be expensive to
parse PF targets on both preview and save in SpecialMassMessage.
*
* @param Title $spamlist
* @param bool $normalize Whether to normalize and deduplicate the
targets
@@ -115,10 +114,12 @@
/** @var Title $member */
foreach ( $members as $member ) {
- $targets[] = [
- 'title' => $member->getPrefixedText(),
- 'wiki' => wfWikiID(),
- 'site' => UrlHelper::getBaseUrl(
$wgCanonicalServer ),
+ $target = new Target();
+ $target->title = $member->getPrefixedText();
+ $target->wiki = wfWikiID();
+ $target->site = UrlHelper::getBaseUrl(
$wgCanonicalServer );
+ $targets = [
+ 'target' => $target,
];
}
return $targets;
@@ -134,11 +135,11 @@
$targets = Revision::newFromTitle( $spamlist
)->getContent()->getValidTargets();
foreach ( $targets as &$target ) {
- if ( array_key_exists( 'site', $target ) ) {
- $target['wiki'] = DatabaseLookup::getDBName(
$target['site'] );
+ if ( /*array_key_exists( 'site', $target )*/ ) {//TODO:
check $target->site exist
+ $target->wiki = DatabaseLookup::getDBName(
$target->site );
} else {
- $target['site'] = UrlHelper::getBaseUrl(
$wgCanonicalServer );
- $target['wiki'] = wfWikiId();
+ $target->site = UrlHelper::getBaseUrl(
$wgCanonicalServer );
+ $target->wiki = wfWikiId();
}
}
return $targets;
diff --git a/includes/Target.php b/includes/Target.php
new file mode 100644
index 0000000..28407a7
--- /dev/null
+++ b/includes/Target.php
@@ -0,0 +1,15 @@
+<?php
+
+namespace MediaWiki\MassMessage;
+
+class Target {
+
+ public $title;
+ public $site;
+ public $wiki;
+ public $error = [];
+
+ function __constructor(){
+
+ }
+}
diff --git a/includes/content/MassMessageListContent.php
b/includes/content/MassMessageListContent.php
index 592d66c..9d4ac81 100644
--- a/includes/content/MassMessageListContent.php
+++ b/includes/content/MassMessageListContent.php
@@ -20,16 +20,16 @@
protected $description;
/**
- * Array of target pages
- * @var array|null
- */
- protected $targets;
-
- /**
* Whether $description and $targets have been populated
* @var bool
*/
protected $decoded = false;
+
+ /**
+ * object of target pages
+ * @var object|null
+ */
+ protected $targets;
/**
* @param string $text
@@ -44,13 +44,13 @@
*/
public function isValid() {
$this->decode(); // Populate $this->description and
$this->targets.
- if ( !is_string( $this->description ) || !is_array(
$this->targets ) ) {
+ if ( !is_string( $this->description ) || !is_object(
$this->targets ) ) {
return false;
}
foreach ( $this->getTargets() as $target ) {
- if ( !is_array( $target )
- || !array_key_exists( 'title', $target )
- || Title::newFromText( $target['title'] ) ===
null
+ if ( !is_object( $target )
+ || !$target->title
+ || $target->title === null
) {
return false;
}
@@ -75,14 +75,14 @@
}
/**
- * @return array
+ * @return object
*/
public function getTargets() {
$this->decode();
- if ( is_array( $this->targets ) ) {
+ if ( is_object( $this->targets ) ) {
return $this->targets;
}
- return [];
+ return $target = new Target();
}
/**
@@ -95,9 +95,9 @@
$targets = $this->getTargets();
$validTargets = [];
foreach ( $targets as $target ) {
- if ( !array_key_exists( 'site', $target )
+ if ( !$target->site
|| $wgAllowGlobalMessaging
- && DatabaseLookup::getDBName( $target['site'] )
!== null
+ && DatabaseLookup::getDBName( $target->site )
!== null
) {
$validTargets[] = $target;
}
@@ -115,14 +115,14 @@
$targets = $this->getTargets();
$targetStrings = [];
foreach ( $targets as $target ) {
- if ( array_key_exists( 'site', $target ) ) {
- $targetStrings[] = $target['title'] . '@' .
$target['site'];
- } elseif ( strpos( $target['title'], '@' ) !== false ) {
+ if ( $target->site ) {
+ $targetStrings[] = $target->title . '@' .
$target->site;
+ } elseif ( strpos( $target->title, '@' ) !== false ) {
// List the site if it'd otherwise be ambiguous
- $targetStrings[] = $target['title'] . '@'
+ $targetStrings[] = $target->title . '@'
. UrlHelper::getBaseUrl(
$wgCanonicalServer );
} else {
- $targetStrings[] = $target['title'];
+ $targetStrings[] = $target->title;
}
}
return $targetStrings;
@@ -140,16 +140,15 @@
if ( $data ) {
$this->description = isset( $data->description ) ?
$data->description : null;
if ( isset( $data->targets ) && is_array(
$data->targets ) ) {
- $this->targets = [];
+ $this->targets = new Target();
foreach ( $data->targets as $target ) {
if ( !is_object( $target ) ) { // There
is a malformed target.
- $this->targets = null;
break;
}
- $this->targets[] = wfObjectToArray(
$target ); // Convert to associative array.
+ $this->targets = wfObjectToArray(
$target );
}
} else {
- $this->targets = null;
+ $this->targets;
}
}
$this->decoded = true;
diff --git a/includes/content/MassMessageListContentHandler.php
b/includes/content/MassMessageListContentHandler.php
index a04f78c..bcfcac7 100644
--- a/includes/content/MassMessageListContentHandler.php
+++ b/includes/content/MassMessageListContentHandler.php
@@ -145,28 +145,28 @@
$site = null;
}
- $result = [];
+ $result = new target();
$title = Title::newFromText( $titleText );
if ( !$title
|| $title->getText() === ''
|| !$title->canExist()
) {
- $result['errors'][] = 'invalidtitle';
+ $result->error = 'invalidtitle';
} else {
- $result['title'] = $title->getPrefixedText(); // Use
the canonical form.
+ $result->title = $title->getPrefixedText(); // Use the
canonical form.
}
if ( $site !== null && $site !== UrlHelper::getBaseUrl(
$wgCanonicalServer ) ) {
- if ( !$wgAllowGlobalMessaging ||
DatabaseLookup::getDBName( $site ) === null ) {
- $result['errors'][] = 'invalidsite';
+ if ( !$wgAllowGlobalMessaging ||
DatabaseLookup::getDBName( $result->site ) === null ) {
+ $result->error = 'invalidsite';
} else {
- $result['site'] = $site;
+ $result->site = $site;
}
} elseif ( $title && $title->isExternal() ) {
// Target has site set to current wiki, but external
title
// TODO: Provide better error message?
- $result['errors'][] = 'invalidtitle';
+ $result->error = 'invalidtitle';
}
return $result;
diff --git a/tests/phpunit/content/MassMessageContentTest.php
b/tests/phpunit/content/MassMessageContentTest.php
index 9c77d49..12b0916 100644
--- a/tests/phpunit/content/MassMessageContentTest.php
+++ b/tests/phpunit/content/MassMessageContentTest.php
@@ -58,15 +58,15 @@
* @covers \Mediawiki\MassMessage\MassMessageListContent::getTargets
*/
public function testGetTargets() {
- $text = '{"description":"","targets":['
- . '{"title":"A"},'
- . '{"title":"B","site":"en.wikipedia.org"}'
- . ']}';
+ $text = '{"description":"","targets->title":"A"},'
+ . '{"targets->title":"B"},'
+ . '{"target->site":"en.wikipedia.org"}'
+ . '}';
$content = new MassMessageListContent( $text );
- $expected = [
- [ 'title' => 'A' ],
- [ 'title' => 'B', 'site' => 'en.wikipedia.org' ]
- ];
+ $expected = new Target();
+ $expected->title = 'A';
+ $expected->title = 'B'; $expected->site = 'en.wikipedia.org';
+
$this->assertEquals( $expected, $content->getTargets() );
}
--
To view, visit https://gerrit.wikimedia.org/r/405222
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2c7fb5d30fd20bf06393cced73027089f2533b6e
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MassMessage
Gerrit-Branch: master
Gerrit-Owner: Noella94 <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
