Ed Hoo has uploaded a new change for review.
https://gerrit.wikimedia.org/r/261317
Change subject: Use parser options to provide context to CargoStore persistence
......................................................................
Use parser options to provide context to CargoStore persistence
This improves thread safety preventing conflicts between saving individual
pages and Recreate Data jobs. Note however that duplicate rows mat still be
created if (a) multiple saves for the same page are taking place and the delete
of 2 or more of these updates take place before the inserts, or (b) the page is
on queue to be recreated and it is saved before the recreation job gets to it.
Bug: T122564
Change-Id: Iee7cc92f1dc0fc6e90698adf74bd6a9baaeeff14
---
M Cargo.hooks.php
M CargoPopulateTableJob.php
M CargoUtils.php
M maintenance/cargoRecreateData.php
M parserfunctions/CargoStore.php
5 files changed, 72 insertions(+), 30 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Cargo
refs/changes/17/261317/1
diff --git a/Cargo.hooks.php b/Cargo.hooks.php
index 9abbb8c..f0dd488 100644
--- a/Cargo.hooks.php
+++ b/Cargo.hooks.php
@@ -174,8 +174,7 @@
// Even though the page will get parsed again after the save,
// we need to parse it here anyway, for the settings we
// added to remain set.
- CargoStore::$settings['origin'] = 'page save';
- CargoUtils::parsePageForStorage( $article->getTitle(),
$content->getNativeData() );
+ CargoUtils::parsePageForStorage( $article->getTitle(),
$content->getNativeData(), 'PageSave' );
return true;
}
@@ -190,7 +189,7 @@
// this setting will be enough to get the correct revision of
// this page to be saved by Cargo, since the page will be
// parsed right after this.
- CargoStore::$settings['origin'] = 'Approved Revs revision
approved';
+ CargoStore::setParserContext($parser,
'ApprovedRevsRevisionApproved');
return true;
}
@@ -201,7 +200,7 @@
$pageID = $title->getArticleID();
self::deletePageFromSystem( $pageID );
// This is all we need - see onARRevisionApproved(), above.
- CargoStore::$settings['origin'] = 'Approved Revs revision
unapproved';
+ CargoStore::setParserContext($parser,
'ApprovedRevsRevisionUnapproved');
return true;
}
diff --git a/CargoPopulateTableJob.php b/CargoPopulateTableJob.php
index 8f0499c..8abec90 100644
--- a/CargoPopulateTableJob.php
+++ b/CargoPopulateTableJob.php
@@ -42,17 +42,10 @@
$cdb->delete( $this->params['dbTableName'], array(
'_pageID' => $article->getID() ) );
}
- // All we need to do here is set some global variables based
+ // All we need to do here is set some parser variables based
// on the parameters of this job, then parse the page -
// the #cargo_store function will take care of the rest.
- CargoStore::$settings['origin'] = 'template';
- CargoStore::$settings['dbTableName'] =
$this->params['dbTableName'];
- CargoUtils::parsePageForStorage( $this->title,
$article->getContent() );
-
- // We need to unset this, if the job was called via runJobs.php,
- // so that it doesn't affect other (non-Cargo) jobs, like page
- // refreshes.
- unset( CargoStore::$settings['origin'] );
+ CargoUtils::parsePageForStorage( $this->title,
$article->getContent(), 'Template', $this->params['dbTableName'] );
wfProfileOut( __METHOD__ );
return true;
diff --git a/CargoUtils.php b/CargoUtils.php
index e1ad9fd..9c33263 100644
--- a/CargoUtils.php
+++ b/CargoUtils.php
@@ -360,7 +360,7 @@
return $value;
}
- public static function parsePageForStorage( $title, $pageContents ) {
+ public static function parsePageForStorage( $title, $pageContents,
$savingContext, $populatedTable = false ) {
// @TODO - is there a "cleaner" way to get a page to be parsed?
global $wgParser;
@@ -375,7 +375,10 @@
} else {
$pageText = $pageContents;
}
- $wgParser->parse( $pageText, $title, new ParserOptions() );
+
+ $parserOptions = new ParserOptions();
+ CargoStore::setParserOptionsContext( $parserOptions,
$savingContext, $populatedTable );
+ $wgParser->parse( $pageText, $title, $parserOptions );
}
/**
diff --git a/maintenance/cargoRecreateData.php
b/maintenance/cargoRecreateData.php
index c7e19ae..b0fc9c6 100644
--- a/maintenance/cargoRecreateData.php
+++ b/maintenance/cargoRecreateData.php
@@ -114,15 +114,13 @@
}
foreach( $titlesWithThisTemplate as $title ) {
- // All we need to do here is set some
global variables based
+ // All we need to do here is set some
parser variables based
// on the parameters of this job, then
parse the page -
// the #cargo_store function will take
care of the rest.
- CargoStore::$settings['origin'] =
'template';
- CargoStore::$settings['dbTableName'] =
$tableName;
$wikiPage = WikiPage::newFromID(
$title->getArticleID() );
$content = $wikiPage->getContent();
$contentText =
ContentHandler::getContentText( $content );
- CargoUtils::parsePageForStorage(
$title, $contentText );
+ CargoUtils::parsePageForStorage(
$title, $contentText, 'Template', $tableName );
}
$offset += 500;
} while ( count( $titlesWithThisTemplate ) == 500 );
diff --git a/parserfunctions/CargoStore.php b/parserfunctions/CargoStore.php
index 2c4c49e..41e0129 100644
--- a/parserfunctions/CargoStore.php
+++ b/parserfunctions/CargoStore.php
@@ -8,7 +8,7 @@
class CargoStore {
- public static $settings = array();
+ private static $settings = array();
const DATE_AND_TIME = 0;
const DATE_ONLY = 1;
@@ -28,11 +28,11 @@
// This function does actual DB modifications - so only proceed
// is this is called via either a page save or a "recreate
// data" action for a template that this page calls.
- if ( count( self::$settings ) == 0 ) {
+
+ $populatedTable = false;
+ $savingContext = self::getContext( $parser->getOptions(),
$populatedTable );
+ if ( ! $savingContext ) {
wfDebugLog( 'cargo', "CargoStore::run() - skipping.\n"
);
- return;
- } elseif ( !array_key_exists( 'origin', self::$settings ) ) {
- wfDebugLog( 'cargo', "CargoStore::run() - skipping
2.\n" );
return;
}
@@ -79,14 +79,10 @@
}
}
- if ( $tableName == '' ) {
- return;
- }
-
- if ( self::$settings['origin'] == 'template' ) {
+ if ( $savingContext == 'Template' ) {
// It came from a template "recreate data" action -
// make sure it passes various criteria.
- if ( self::$settings['dbTableName'] != $tableName ) {
+ if ( $populatedTable != $tableName ) {
wfDebugLog( 'cargo', "CargoStore::run() -
skipping 4.\n" );
return;
}
@@ -331,4 +327,57 @@
}
}
+ /**
+ * Set the context for the CargoStore::run callback
+ * The context is set by adding extra keys to the parser options
+ */
+ public static function setParserContext(&$parser, $savingContext,
$populatedTable = false ) {
+ if ( $savingContext ) {
+
$parser->getOptions()->addExtraKey('CargoOrigin_'.$savingContext);
+ }
+ if ( $populatedTable ) {
+
$parser->getOptions()->addExtraKey('CargoTable_'.$populatedTable);
+ }
+ }
+
+ /**
+ * Set the context for the CargoStore::run callback
+ * The context is set by adding extra keys to the parser options
+ */
+ public static function setParserOptionsContext(&$parserOptions,
$savingContext, $populatedTable = false ) {
+ if ( $savingContext ) {
+
$parserOptions->addExtraKey('CargoOrigin_'.$savingContext);
+ }
+ if ( $populatedTable ) {
+
$parserOptions->addExtraKey('CargoTable_'.$populatedTable);
+ }
+ }
+
+ /**
+ * Retrieves the context for the CargoStore::run callback
+ * The context is set by adding extra keys to the parser options
+ */
+ public static function getContext($parserOptions, &$populatedTable ) {
+ $options = explode( '!', $parserOptions->optionsHash( Array() )
);
+ $savingContext = false;
+ $populatedTable = false;
+
+ for ( $i =0 ; $i < count($options) ; $i++) {
+ $thisOption = $options[$i];
+ if ( substr( $thisOption, 0, 5 ) == 'Cargo' ) {
+ $key = explode( '_', $thisOption )[0];
+ $value = substr( $thisOption, strlen($key) + 1
);
+ switch ( $key ) {
+ case 'CargoOrigin':
+ $savingContext = $value;
+ break;
+ case 'CargoTable':
+ $populatedTable = $value;
+ break;
+ }
+ }
+ }
+ return $savingContext;
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/261317
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iee7cc92f1dc0fc6e90698adf74bd6a9baaeeff14
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Cargo
Gerrit-Branch: master
Gerrit-Owner: Ed Hoo <edward....@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
