jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/389202 )
Change subject: DifferenceEngine: Improve cache invalidation
......................................................................
DifferenceEngine: Improve cache invalidation
Invalidate the diff cache if the engine producing the diff changes, or
if a configuration setting that controls the diff output changes. This
is probably what most users expect, that changing the configuration will
result in a change for diffs that may have already been viewed.
For wikidiff2 specifically, a change in version or
$wgWikiDiff2MovedParagraphDetectionCutoff will invalidate the cache.
Refactor engine detection and sanity-checking into a private getEngine()
function.
As part of this getDiffBodyCacheKey() was deprecated, and subclasses
should implement getDiffBodyCacheKeyParams() instead. Drop the
deprecated and unused MW_DIFF_VERSION constant while we're at it, and
bump DIFF_VERSION since we're already changing the cache key format.
Bug: T180043
Change-Id: I4e386ca05bd2a2fb54208d760c131eb42e3a72ab
---
M RELEASE-NOTES-1.31
M includes/diff/DifferenceEngine.php
2 files changed, 82 insertions(+), 22 deletions(-)
Approvals:
Addshore: Looks good to me, approved
jenkins-bot: Verified
diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31
index 8caab05..f1fd9d3 100644
--- a/RELEASE-NOTES-1.31
+++ b/RELEASE-NOTES-1.31
@@ -98,6 +98,10 @@
* Revision::setUserIdAndName() was deprecated.
* Access to TitleValue class properties was deprecated, the relevant getters
should be used instead.
+* DifferenceEngine::getDiffBodyCacheKey() is deprecated. Subclasses should
+ override DifferenceEngine::getDiffBodyCacheKeyParams() instead.
+* The deprecated MW_DIFF_VERSION constant was removed.
+ DifferenceEngine::MW_DIFF_VERSION should be used instead.
== Compatibility ==
MediaWiki 1.31 requires PHP 5.5.9 or later. There is experimental support for
diff --git a/includes/diff/DifferenceEngine.php
b/includes/diff/DifferenceEngine.php
index 813ee08..fb12c1e 100644
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -23,9 +23,6 @@
use MediaWiki\MediaWikiServices;
use MediaWiki\Shell\Shell;
-/** @deprecated use class constant instead */
-define( 'MW_DIFF_VERSION', '1.11a' );
-
/**
* @todo document
* @ingroup DifferenceEngine
@@ -37,7 +34,7 @@
* fixes important bugs or such to force cached diff views to
* clear.
*/
- const DIFF_VERSION = MW_DIFF_VERSION;
+ const DIFF_VERSION = '1.12';
/** @var int */
public $mOldid;
@@ -753,7 +750,15 @@
$key = false;
$cache = ObjectCache::getMainWANInstance();
if ( $this->mOldid && $this->mNewid ) {
+ // Check if subclass is still using the old way
+ // for backwards-compatibility
$key = $this->getDiffBodyCacheKey();
+ if ( $key === null ) {
+ $key = call_user_func_array(
+ [ $cache, 'makeKey' ],
+ $this->getDiffBodyCacheKeyParams()
+ );
+ }
// Try cache
if ( !$this->mRefreshCache ) {
@@ -799,18 +804,49 @@
/**
* Returns the cache key for diff body text or content.
*
+ * @deprecated since 1.31, use getDiffBodyCacheKeyParams() instead
* @since 1.23
*
* @throws MWException
- * @return string
+ * @return string|null
*/
protected function getDiffBodyCacheKey() {
+ return null;
+ }
+
+ /**
+ * Get the cache key parameters
+ *
+ * Subclasses can replace the first element in the array to something
+ * more specific to the type of diff (e.g. "inline-diff"), or append
+ * if the cache should vary on more things. Overriding entirely should
+ * be avoided.
+ *
+ * @since 1.31
+ *
+ * @return array
+ * @throws MWException
+ */
+ protected function getDiffBodyCacheKeyParams() {
if ( !$this->mOldid || !$this->mNewid ) {
throw new MWException( 'mOldid and mNewid must be set
to get diff cache key.' );
}
- return wfMemcKey( 'diff', 'version', self::DIFF_VERSION,
- 'oldid', $this->mOldid, 'newid', $this->mNewid );
+ $engine = $this->getEngine();
+ $params = [
+ 'diff',
+ $engine,
+ self::DIFF_VERSION,
+ "old-{$this->mOldid}",
+ "rev-{$this->mNewid}"
+ ];
+
+ if ( $engine === 'wikidiff2' ) {
+ $params[] = phpversion( 'wikidiff2' );
+ $params[] = $this->getConfig()->get(
'WikiDiff2MovedParagraphDetectionCutoff' );
+ }
+
+ return $params;
}
/**
@@ -897,19 +933,14 @@
}
/**
- * Generates diff, to be wrapped internally in a logging/instrumentation
+ * Process $wgExternalDiffEngine and get a sane, usable engine
*
- * @param string $otext Old text, must be already segmented
- * @param string $ntext New text, must be already segmented
- * @return bool|string
- * @throws Exception
+ * @return bool|string 'wikidiff2', path to an executable, or false
*/
- protected function textDiff( $otext, $ntext ) {
- global $wgExternalDiffEngine, $wgContLang;
-
- $otext = str_replace( "\r\n", "\n", $otext );
- $ntext = str_replace( "\r\n", "\n", $ntext );
-
+ private function getEngine() {
+ global $wgExternalDiffEngine;
+ // We use the global here instead of Config because we write to
the value,
+ // and Config is not mutable.
if ( $wgExternalDiffEngine == 'wikidiff' ||
$wgExternalDiffEngine == 'wikidiff3' ) {
wfDeprecated( "\$wgExternalDiffEngine =
'{$wgExternalDiffEngine}'", '1.27' );
$wgExternalDiffEngine = false;
@@ -922,9 +953,34 @@
$wgExternalDiffEngine = false;
}
+ if ( is_string( $wgExternalDiffEngine ) && is_executable(
$wgExternalDiffEngine ) ) {
+ return $wgExternalDiffEngine;
+ } elseif ( $wgExternalDiffEngine === false && function_exists(
'wikidiff2_do_diff' ) ) {
+ return 'wikidiff2';
+ } else {
+ // Native PHP
+ return false;
+ }
+ }
+
+ /**
+ * Generates diff, to be wrapped internally in a logging/instrumentation
+ *
+ * @param string $otext Old text, must be already segmented
+ * @param string $ntext New text, must be already segmented
+ * @return bool|string
+ */
+ protected function textDiff( $otext, $ntext ) {
+ global $wgContLang;
+
+ $otext = str_replace( "\r\n", "\n", $otext );
+ $ntext = str_replace( "\r\n", "\n", $ntext );
+
+ $engine = $this->getEngine();
+
// Better external diff engine, the 2 may some day be dropped
// This one does the escaping and segmenting itself
- if ( function_exists( 'wikidiff2_do_diff' ) &&
$wgExternalDiffEngine === false ) {
+ if ( $engine === 'wikidiff2' ) {
$wikidiff2Version = phpversion( 'wikidiff2' );
if (
$wikidiff2Version !== false &&
@@ -954,7 +1010,7 @@
$text .= $this->debug( 'wikidiff2' );
return $text;
- } elseif ( $wgExternalDiffEngine !== false && is_executable(
$wgExternalDiffEngine ) ) {
+ } elseif ( $engine !== false ) {
# Diff via the shell
$tmpDir = wfTempDir();
$tempName1 = tempnam( $tmpDir, 'diff_' );
@@ -972,7 +1028,7 @@
fwrite( $tempFile2, $ntext );
fclose( $tempFile1 );
fclose( $tempFile2 );
- $cmd = [ $wgExternalDiffEngine, $tempName1, $tempName2
];
+ $cmd = [ $engine, $tempName1, $tempName2 ];
$result = Shell::command( $cmd )
->execute();
$exitCode = $result->getExitCode();
@@ -982,7 +1038,7 @@
);
}
$difftext = $result->getStdout();
- $difftext .= $this->debug( "external
$wgExternalDiffEngine" );
+ $difftext .= $this->debug( "external $engine" );
unlink( $tempName1 );
unlink( $tempName2 );
--
To view, visit https://gerrit.wikimedia.org/r/389202
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4e386ca05bd2a2fb54208d760c131eb42e3a72ab
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Jkroll <johannes.kr...@wikimedia.de>
Gerrit-Reviewer: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: MaxSem <maxsem.w...@gmail.com>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.kr...@wikimedia.de>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: WMDE-Fisch <christoph.jau...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
