Anja Jentzsch has uploaded a new change for review.
https://gerrit.wikimedia.org/r/54850
Change subject: Fix namespace filtering in ChangeHandler.
......................................................................
Fix namespace filtering in ChangeHandler.
In addition to the actual fix (include/exclude params in the correct order),
thich change includes a number of improvements to debug logging and
error reporting during change handling on the client.
Note: this needs to be backported to the wmf12 branch.
Change-Id: Ib8ac99f762b9aed5f61d62c10bf7706ed70affa8
---
M client/includes/ChangeHandler.php
M client/includes/NamespaceChecker.php
M client/includes/WikiPageUpdater.php
M lib/includes/ChangeNotificationJob.php
M lib/includes/changes/ItemChange.php
M lib/maintenance/dispatchChanges.php
6 files changed, 53 insertions(+), 23 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/50/54850/1
diff --git a/client/includes/ChangeHandler.php
b/client/includes/ChangeHandler.php
index 4ad7741..3aa21ed 100644
--- a/client/includes/ChangeHandler.php
+++ b/client/includes/ChangeHandler.php
@@ -141,8 +141,8 @@
// TODO: allow these to be passed in as parameters!
$this->setNamespaces(
- Settings::get( 'excludeNamespaces' ),
- Settings::get( 'namespaces' )
+ Settings::get( 'namespaces' ),
+ Settings::get( 'excludeNamespaces' )
);
$this->injectRC = Settings::get( 'injectRecentChanges' );
@@ -237,7 +237,10 @@
$minor = true;
$bot = true;
+ $ids = array();
+
foreach ( $changes as $change ) {
+ $ids[] = $change->getId();
$meta = $change->getMetadata();
$minor &= isset( $meta['minor'] ) &&
(bool)$meta['minor'];
@@ -289,6 +292,7 @@
) );
$info = $change->hasField( 'info' ) ? $change->getField( 'info'
) : array();
+ $info['change-ids'] = $ids;
$info['changes'] = $changes;
$change->setField( 'info', $info );
@@ -397,6 +401,10 @@
}
usort( $coalesced,
'Wikibase\ChangeHandler::compareChangesByTimestamp' );
+
+ wfDebugLog( __CLASS__, __METHOD__ . ": coalesced "
+ . count( $changes ) . " into " . count( $coalesced ) .
" changes" );
+
return $coalesced;
}
@@ -466,7 +474,10 @@
*/
public function handleChange( Change $change ) {
wfProfileIn( __METHOD__ );
- wfDebugLog( __CLASS__, __FUNCTION__ . ": handling change #" .
$change->getId() );
+
+ $chid = self::getChangeIdForLog( $change );
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": handling change #$chid"
+ . " (" . $change->getType() . ")" );
//TODO: Actions may be per-title, depending on how the change
applies to that page.
// We'll need on list of titles per action.
@@ -474,6 +485,7 @@
if ( $actions === 0 ) {
// nothing to do
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": No actions to
take for change #$chid." );
wfProfileOut( __METHOD__ );
return false;
}
@@ -487,11 +499,13 @@
if ( empty( $titlesToUpdate ) ) {
// nothing to do
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": No pages to
update for change #$chid." );
wfProfileOut( __METHOD__ );
return false;
}
- wfDebugLog( __CLASS__, __FUNCTION__ . ": " . count(
$titlesToUpdate ) . " pages to update." );
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": updating " . count(
$titlesToUpdate )
+ . " pages (actions: " . dechex( $actions ). ") for
change #$chid." );
$this->updatePages( $change, $actions, $titlesToUpdate );
@@ -597,7 +611,7 @@
if ( $rcAttribs !== false ) {
$this->updater->injectRCRecord( $title,
$rcAttribs );
} else {
- trigger_error( "change #" .
$change->getId() . " did not provide RC info", E_USER_WARNING );
+ trigger_error( "change #" .
self::getChangeIdForLog( $change ) . " did not provide RC info", E_USER_WARNING
);
}
}
@@ -606,6 +620,26 @@
}
wfProfileOut( __METHOD__ );
+ }
+
+ /**
+ * Returns a human readable change ID, containing multiple IDs in case
of a
+ * coalesced change.
+ *
+ * @param Change $change
+ *
+ * @return string
+ */
+ protected static function getChangeIdForLog( Change $change ) {
+ $fields = $change->getFields(); //@todo: add getFields() to the
interface, or provide getters!
+
+ if ( isset( $fields['info']['change-ids'] ) ) {
+ $chid = implode( '|', $fields['info']['change-ids'] );
+ } else {
+ $chid = $change->getId();
+ }
+
+ return $chid;
}
/**
@@ -649,16 +683,6 @@
$params = array(
'wikibase-repo-change' => array_merge( $fields, $rcinfo
)
);
-
- //XXX: The same change may be reported to several target pages;
- // The comment we generate should be adapted to the role
that page
- // plays in the change, e.g. when a sitelink changes from
one page to another,
- // the link was effectively removed from one and added to
the other page.
- $rc = ExternalRecentChange::newFromAttribs( $params, $title );
-
- // @todo batch these
- wfDebugLog( __CLASS__, __FUNCTION__ . ": saving RC entry for "
. $title->getFullText() );
- $rc->save();
wfProfileOut( __METHOD__ );
return $params;
diff --git a/client/includes/NamespaceChecker.php
b/client/includes/NamespaceChecker.php
index f91c97e..4d00d8a 100644
--- a/client/includes/NamespaceChecker.php
+++ b/client/includes/NamespaceChecker.php
@@ -59,8 +59,7 @@
*/
public function isWikibaseEnabled( $namespace ) {
if( !is_int( $namespace ) ) {
- wfDebug( 'Invalid namespace in Wikibase namespace
checker.' );
- return false;
+ throw new \MWException( __METHOD__ . " expected a
namespace ID." );
}
if ( $this->excludedNamespaces !== array() ) {
diff --git a/client/includes/WikiPageUpdater.php
b/client/includes/WikiPageUpdater.php
index 0bb1aa1..d5306e6 100644
--- a/client/includes/WikiPageUpdater.php
+++ b/client/includes/WikiPageUpdater.php
@@ -59,7 +59,7 @@
public function purgeWebCache( array $titles ) {
/* @var \Title $title */
foreach ( $titles as $title ) {
- wfDebugLog( __CLASS__, __FUNCTION__ . ": purging page "
. $title->getText() );
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": purging web
cache for " . $title->getText() );
$title->purgeSquid();
}
}
@@ -74,7 +74,8 @@
public function scheduleRefreshLinks( array $titles ) {
/* @var \Title $title */
foreach ( $titles as $title ) {
- wfDebugLog( __CLASS__, __FUNCTION__ . ": scheduling
page " . $title->getText() );
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": scheduling
refresh links for "
+ . $title->getText() );
//XXX: use \RefreshLinksJob2 ?!
$job = new \RefreshLinksJob(
diff --git a/lib/includes/ChangeNotificationJob.php
b/lib/includes/ChangeNotificationJob.php
index dde7f2e..85150bc 100644
--- a/lib/includes/ChangeNotificationJob.php
+++ b/lib/includes/ChangeNotificationJob.php
@@ -117,10 +117,14 @@
$table =
ClientStoreFactory::getStore()->newChangesTable();
$this->changes = $table->selectObjects( null, array(
'id' => $ids ), array(), __METHOD__ );
- wfDebugLog( __CLASS__, __FUNCTION__ . ": loaded " .
count( $this->changes ) . " changes." );
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": loaded " .
count( $this->changes )
+ . " of " . count( $ids ) . " changes." );
if ( count( $this->changes ) != count( $ids ) ) {
- wfWarn( "Number of changes loaded mismatches
the number of change IDs provided." );
+ trigger_error( "Number of changes loaded
mismatches the number of change IDs provided: "
+ . count( $this->changes ) . " != " .
count( $ids ) . ". "
+ . " Some changes were lost, possibly
due to premature pruning.",
+ E_USER_WARNING );
}
}
diff --git a/lib/includes/changes/ItemChange.php
b/lib/includes/changes/ItemChange.php
index d3bf23a..51b327f 100644
--- a/lib/includes/changes/ItemChange.php
+++ b/lib/includes/changes/ItemChange.php
@@ -42,7 +42,9 @@
// obsolete instances in the database, etc.
$cls = $diff === null ? 'null' : get_class( $diff );
- trigger_error( 'Cannot get sitelink diff from ' . $cls
. '.', E_USER_WARNING );
+ trigger_error(
+ 'Cannot get sitelink diff from ' . $cls . '.
Change # ' . $this->getId()
+ . ", type " . $this->getType(), E_USER_WARNING
);
return new \Diff\Diff();
} else {
diff --git a/lib/maintenance/dispatchChanges.php
b/lib/maintenance/dispatchChanges.php
index 78663b5..53d706e 100644
--- a/lib/maintenance/dispatchChanges.php
+++ b/lib/maintenance/dispatchChanges.php
@@ -258,7 +258,7 @@
$n = count( $changes );
if ( $n === 0 ) {
- $this->log( "Posted no changes to $wikiDB (nothing to
do). "
+ $this->trace( "Posted no changes to $wikiDB (nothing to
do). "
. "Next ID is $continueAfter."
);
} else {
/* @var Change $last */
--
To view, visit https://gerrit.wikimedia.org/r/54850
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib8ac99f762b9aed5f61d62c10bf7706ed70affa8
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: mw1.21-wmf12
Gerrit-Owner: Anja Jentzsch <anja.jentz...@wikimedia.de>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
