Mattflaschen has uploaded a new change for review.
https://gerrit.wikimedia.org/r/211847
Change subject: Only enforce subpage rules on batch wikitext->Flow conversions
......................................................................
Only enforce subpage rules on batch wikitext->Flow conversions
Refactor isAllowed to move the subpage rules to ConversionStrategy
Also move logic about LQT vs. wikitext (and which conversions can
handle which) to the same place.
Bug: T99111
Change-Id: I0fb1b2e3bbca97802520f54d2ed864edb482f414
(cherry picked from commit 95148ad059f09b586dc89155b1924545aa20ee5f)
---
M autoload.php
M includes/Import/Converter.php
A includes/Import/EnableFlow/EnableFlowWikitextConversionStrategy.php
M includes/Import/IConversionStrategy.php
M includes/Import/LiquidThreadsApi/ConversionStrategy.php
M includes/Import/Wikitext/ConversionStrategy.php
M includes/Specials/SpecialEnableFlow.php
M maintenance/convertNamespaceFromWikitext.php
M tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
9 files changed, 194 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/47/211847/1
diff --git a/autoload.php b/autoload.php
index 975f25f..4634e3e 100644
--- a/autoload.php
+++ b/autoload.php
@@ -162,6 +162,7 @@
'Flow\\Formatter\\TopicListQuery' => __DIR__ .
'/includes/Formatter/TopicListQuery.php',
'Flow\\Formatter\\TopicRow' => __DIR__ .
'/includes/Formatter/TopicRow.php',
'Flow\\Import\\Converter' => __DIR__ . '/includes/Import/Converter.php',
+ 'Flow\\Import\\EnableFlow\\EnableFlowWikitextConversionStrategy' =>
__DIR__ .
'/includes/Import/EnableFlow/EnableFlowWikitextConversionStrategy.php',
'Flow\\Import\\FileImportSourceStore' => __DIR__ .
'/includes/Import/ImportSourceStore.php',
'Flow\\Import\\HistoricalUIDGenerator' => __DIR__ .
'/includes/Import/Importer.php',
'Flow\\Import\\IConversionStrategy' => __DIR__ .
'/includes/Import/IConversionStrategy.php',
diff --git a/includes/Import/Converter.php b/includes/Import/Converter.php
index 5e1e6cb..1d71221 100644
--- a/includes/Import/Converter.php
+++ b/includes/Import/Converter.php
@@ -155,15 +155,8 @@
return true;
}
- // Don't allow conversion of sub pages unless it is
- // a talk page with matching subject page. For example
- // we will convert User_talk:Foo/bar only if User:Foo/bar
- // exists, and we will never convert User:Baz/bang.
- if ( $title->isSubPage() && ( !$title->isTalkPage() ||
!$title->getSubjectPage()->exists() ) ) {
- return false;
- }
-
- return true;
+ // Finally, check strategy-specific logic
+ return $this->strategy->shouldConvert( $title );
}
protected function doConversion( Title $title, Title $movedFrom = null
) {
diff --git
a/includes/Import/EnableFlow/EnableFlowWikitextConversionStrategy.php
b/includes/Import/EnableFlow/EnableFlowWikitextConversionStrategy.php
new file mode 100644
index 0000000..f8bea89
--- /dev/null
+++ b/includes/Import/EnableFlow/EnableFlowWikitextConversionStrategy.php
@@ -0,0 +1,18 @@
+<?php
+
+namespace Flow\Import\EnableFlow;
+
+use Title;
+
+class EnableFlowWikitextConversionStrategy extends
\Flow\Import\Wikitext\ConversionStrategy {
+ /**
+ * {@inheritDoc}
+ */
+ public function meetsSubpageRequirements( Title $sourceTitle ) {
+ // If they're using Special:EnableFlow, they're choosing a
specific page
+ // one at a time, so assume they know what they're doing. This
allows:
+ // * Namespace_talk:Foo/bar even if Namespace:Foo/bar does not
exist
+ // * Namespace:Baz/bang
+ return true;
+ }
+}
diff --git a/includes/Import/IConversionStrategy.php
b/includes/Import/IConversionStrategy.php
index c797da2..a035a2c 100644
--- a/includes/Import/IConversionStrategy.php
+++ b/includes/Import/IConversionStrategy.php
@@ -9,6 +9,7 @@
/**
* Interface between the Converter and an implementation of IImportSource.
+ * Informs conversion behavior and which pages should be converted.
*/
interface IConversionStrategy {
/**
@@ -79,4 +80,14 @@
* @return Postprocessor|null
*/
function getPostprocessor();
+
+ /**
+ * Checks whether the title should be converted.
+ * This is a secondary filter in addition to the original list (which
might be a
+ * single title, a namespace, etc.) passed into convert/convertAll.
+ *
+ * @param Title $sourceTitle
+ * @return bool True if and only if it should be converted
+ */
+ function shouldConvert( Title $sourceTitle );
}
diff --git a/includes/Import/LiquidThreadsApi/ConversionStrategy.php
b/includes/Import/LiquidThreadsApi/ConversionStrategy.php
index be42ccf..74bd12b 100644
--- a/includes/Import/LiquidThreadsApi/ConversionStrategy.php
+++ b/includes/Import/LiquidThreadsApi/ConversionStrategy.php
@@ -143,4 +143,12 @@
return $group;
}
+
+ /**
+ * {@inheritDoc}
+ */
+ public function shouldConvert( Title $sourceTitle ) {
+ // The expensive part of this (user-override checking) is
cached by LQT.
+ return LqtDispatch::isLqtPage( $sourceTitle );
+ }
}
diff --git a/includes/Import/Wikitext/ConversionStrategy.php
b/includes/Import/Wikitext/ConversionStrategy.php
index e4339c6..cfd9bbb 100644
--- a/includes/Import/Wikitext/ConversionStrategy.php
+++ b/includes/Import/Wikitext/ConversionStrategy.php
@@ -8,6 +8,7 @@
use Flow\Import\IConversionStrategy;
use Flow\Import\ImportSourceStore;
use Parser;
+use Psr\Log\LoggerInterface;
use StubObject;
use Title;
use WikitextContent;
@@ -27,6 +28,11 @@
* existing comment.
*/
class ConversionStrategy implements IConversionStrategy {
+ /**
+ * @var LoggerInterface
+ */
+ protected $logger;
+
/**
* @var ImportSourceStore
*/
@@ -54,11 +60,13 @@
public function __construct(
$parser,
ImportSourceStore $sourceStore,
+ LoggerInterface $logger,
$preferredArchiveTitle = null,
$headerSuffix = null
) {
$this->parser = $parser;
$this->sourceStore = $sourceStore;
+ $this->logger = $logger;
$this->headerSuffix = $headerSuffix;
if ( isset( $preferredArchiveTitle ) && !empty(
$preferredArchiveTitle ) ) {
@@ -142,4 +150,44 @@
return new WikitextContent( $newWikitext );
}
+
+ // Public only for unit testing
+ /**
+ * Checks whether it meets the applicable subpage rules. Meant to be
overriden by
+ * subclasses that do not have the same requirements
+ *
+ * @param Title $sourceTitle Title to check
+ * @return bool Whether it meets the applicable subpage requirements
+ */
+ public function meetsSubpageRequirements( $sourceTitle ) {
+ // Don't allow conversion of sub pages unless it is
+ // a talk page with matching subject page. For example
+ // we will convert User_talk:Foo/bar only if User:Foo/bar
+ // exists, and we will never convert User:Baz/bang.
+ if ( $sourceTitle->isSubPage() && ( !$sourceTitle->isTalkPage()
|| !$sourceTitle->getSubjectPage()->exists() ) ) {
+ return false;
+ }
+
+ return true;
+ }
+
+ /**
+ * {@inheritDoc}
+ */
+ public function shouldConvert( Title $sourceTitle ) {
+ // If we have LiquidThreads filter out any pages with that
enabled. They should
+ // be converted separately.
+ if ( class_exists( 'LqtDispatch' ) ) {
+ if ( \LqtDispatch::isLqtPage( $sourceTitle ) ) {
+ $this->logger->info( "Skipping LQT enabled
page, conversion must be done with convertLqt.php or
convertLqtPageOnLocalWiki.php: $sourceTitle" );
+ return false;
+ }
+ }
+
+ if ( !$this->meetsSubpageRequirements( $sourceTitle ) ) {
+ return false;
+ }
+
+ return true;
+ }
}
diff --git a/includes/Specials/SpecialEnableFlow.php
b/includes/Specials/SpecialEnableFlow.php
index 7eaa358..9abfea1 100644
--- a/includes/Specials/SpecialEnableFlow.php
+++ b/includes/Specials/SpecialEnableFlow.php
@@ -7,7 +7,7 @@
use Title;
use Flow\Container;
use Flow\Import\Converter;
-use Flow\Import\Wikitext\ConversionStrategy;
+use Flow\Import\EnableFlow\EnableFlowWikitextConversionStrategy;
use Flow\Import\NullImportSourceStore;
/**
@@ -95,14 +95,17 @@
return Status::newFatal(
'flow-special-enableflow-page-is-liquidthreads', $page );
}
+ $logger = Container::get( 'default_logger' );
+
$converter = new Converter(
wfGetDB( DB_MASTER ),
Container::get( 'importer' ),
- Container::get( 'default_logger' ),
+ $logger,
$this->occupationController->getTalkpageManager(),
- new ConversionStrategy(
+ new EnableFlowWikitextConversionStrategy(
Container::get( 'parser' ),
new NullImportSourceStore(),
+ $logger,
$data['archive-title-format'],
$data['header']
)
diff --git a/maintenance/convertNamespaceFromWikitext.php
b/maintenance/convertNamespaceFromWikitext.php
index 0cbc425..229ffc8 100644
--- a/maintenance/convertNamespaceFromWikitext.php
+++ b/maintenance/convertNamespaceFromWikitext.php
@@ -40,7 +40,8 @@
FlowHooks::getOccupationController()->getTalkpageManager(),
new Flow\Import\Wikitext\ConversionStrategy(
$wgParser,
- new Flow\Import\NullImportSourceStore()
+ new Flow\Import\NullImportSourceStore(),
+ $logger
)
);
@@ -53,19 +54,6 @@
// so we can wrap that.
$it = $it->getIterator();
-
- // if we have liquid threads filter out any pages with that
enabled. They should
- // be converted separately.
- if ( class_exists( 'LqtDispatch' ) ) {
- $it = new CallbackFilterIterator( $it, function( $title
) use ( $logger ) {
- if ( LqtDispatch::isLqtPage( $title ) ) {
- $logger->info( "Skipping LQT enabled
page, conversion must be done with convertLqt.php or convertLqtPage.php:
$title" );
- return false;
- } else {
- return true;
- }
- } );
- }
$converter->convertAll( $it );
}
diff --git a/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
b/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
index 4208a35..293bc7f 100644
--- a/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
+++ b/tests/phpunit/Import/Wikitext/ConversionStrategyTest.php
@@ -2,11 +2,13 @@
namespace Flow\Tests\Import\Wikitext;
+use Flow\Container;
use DateTime;
use DateTimeZone;
use Flow\Import\ImportSourceStore;
use Flow\Import\NullImportSourceStore;
use Flow\Import\Wikitext\ConversionStrategy;
+use LinkCache;
use Parser;
use Title;
use WikitextContent;
@@ -82,6 +84,100 @@
);
}
+ public function testShouldConvertLqt() {
+ if( !class_exists( 'LqtDispatch' ) ) {
+ $this->markTestSkipped( 'LiquidThreads not enabled' );
+ }
+
+ $strategy = $this->createStrategy();
+
+ $lqtPagesName = 'Talk:Some ConversionStrategyTest LQT page';
+ $this->setMwGlobals( array(
+ 'wgLqtNamespaces' => array( NS_HELP_TALK ),
+ 'wgLqtPages' => array( $lqtPagesName ),
+ ) );
+
+ // Not subpage, not LQT
+ $nonLqtTitle = Title::newFromText( 'Talk:Some
ConversionStrategyTest page' );
+ $this->assertSame(
+ true,
+ $strategy->shouldConvert( $nonLqtTitle ),
+ 'Normal non-LQT talk page should be converted'
+ );
+
+ $lqtNamespacesTitle = Title::makeTitle(
+ NS_HELP_TALK,
+ 'Some other ConversionStrategyTest LQT page'
+ );
+ $this->assertSame(
+ false,
+ $strategy->shouldConvert( $lqtNamespacesTitle ),
+ 'LQT wgLqtNamespaces talk page should not be converted'
+ );
+
+ $lqtPagesTitle = Title::newFromText( $lqtPagesName );
+ $this->assertSame(
+ false,
+ $strategy->shouldConvert( $lqtPagesTitle ),
+ 'LQT wgLqtPages talk page should not be converted'
+ );
+
+ }
+ /**
+ * @dataProvider provideMeetsSubpageRequirements
+ */
+ public function testMeetsSubpageRequirements( $pageName,
$expectedResult, $subjectExists, $message ) {
+ $strategy = $this->createStrategy();
+ $title = Title::newFromText( $pageName );
+ $subjectTitle = $title->getSubjectPage();
+ $linkCache = LinkCache::singleton();
+
+ // Fake whether $subjectTitle exists
+ if ( $subjectExists ) {
+ $linkCache->addGoodLinkObj(
+ 1, // Fake article ID
+ $subjectTitle
+ );
+ } else {
+ $linkCache->addBadLinkObj( $subjectTitle );
+ }
+
+ $this->assertSame(
+ $expectedResult,
+ $strategy->meetsSubpageRequirements( $title ),
+ $message
+ );
+ }
+
+ public function provideMeetsSubpageRequirements() {
+ return array(
+ array(
+ 'Talk:Some ConversionStrategyTest page',
+ true,
+ true, // Shouldn't matter
+ 'Non-subpage talk page',
+ ),
+ array(
+ 'Talk:Some/ConversionStrategyTest subpage 1',
+ true,
+ true,
+ 'Talk subpage where subject exists',
+ ),
+ array(
+ 'Talk:Some/ConversionStrategyTest subpage 2',
+ false,
+ false,
+ 'Talk subpage where subject doesn\'t exist',
+ ),
+ array(
+ 'User:Some/ConversionStrategyTest subpage',
+ false,
+ true,
+ 'Existing subpage in subject namespace'
+ ),
+ );
+ }
+
protected function createStrategy(
Parser $parser = null,
ImportSourceStore $sourceStore = null
@@ -90,7 +186,8 @@
return new ConversionStrategy(
$parser ?: $wgParser,
- $sourceStore ?: new NullImportSourceStore
+ $sourceStore ?: new NullImportSourceStore,
+ Container::get( 'default_logger' )
);
}
}
--
To view, visit https://gerrit.wikimedia.org/r/211847
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0fb1b2e3bbca97802520f54d2ed864edb482f414
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: wmf/1.26wmf5
Gerrit-Owner: Mattflaschen <mflasc...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
