Daniel Kinzler has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/356228 )
Change subject: Allow makeEmptyContent to return null for some kinds of content.
......................................................................
Allow makeEmptyContent to return null for some kinds of content.
ContentHandler::makeEmptyContent() is needed to initialize empty pages
for direct editing. Content models that do not support direct editing
may also not support empty content at all (e.g. Wikibase Property pages).
This patch allows makeEmptyContent to return null for such kinds of content.
Change-Id: I028a80db8a54234ab82fd1434c0e1429d50904de
BREAKING: ContentHandler::makeEmptyContent may return null.
---
M RELEASE-NOTES-1.30
M includes/FeedUtils.php
M includes/content/ContentHandler.php
M includes/jobqueue/jobs/RefreshLinksJob.php
M includes/specials/SpecialChangeContentModel.php
M maintenance/cleanupSpam.php
M tests/phpunit/structure/ContentHandlerSanityTest.php
7 files changed, 55 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/28/356228/1
diff --git a/RELEASE-NOTES-1.30 b/RELEASE-NOTES-1.30
index e61277a..f7fffe2 100644
--- a/RELEASE-NOTES-1.30
+++ b/RELEASE-NOTES-1.30
@@ -87,6 +87,8 @@
As a result of the new uniform handling, '-{' may need to be escaped
(for example, as '-<nowiki/>{') where it occurs inside template arguments
or wikilinks.
+* ContentHandler::makeEmptyContent() may now return null for content models
+ that do not support direct editing.
== Compatibility ==
MediaWiki 1.30 requires PHP 5.5.9 or later. There is experimental support for
diff --git a/includes/FeedUtils.php b/includes/FeedUtils.php
index 3268291..020be1f 100644
--- a/includes/FeedUtils.php
+++ b/includes/FeedUtils.php
@@ -165,6 +165,7 @@
} else {
$rev = Revision::newFromId( $newid );
if ( $wgFeedDiffCutoff <= 0 || is_null( $rev ) ) {
+ // NOTE: makeEmptyContent() can return null for
some kinds of content
$newContent = ContentHandler::getForTitle(
$title )->makeEmptyContent();
} else {
$newContent = $rev->getContent();
diff --git a/includes/content/ContentHandler.php
b/includes/content/ContentHandler.php
index bccb147..ddd1075 100644
--- a/includes/content/ContentHandler.php
+++ b/includes/content/ContentHandler.php
@@ -464,9 +464,14 @@
* Creates an empty Content object of the type supported by this
* ContentHandler.
*
- * @since 1.21
+ * @note This method is guaranteed to return a Content object if
supportsDirectEditing()
+ * or supportsDirectApiEditing() returns true. If both of these methods
return false
+ * (meaning no direct editing is supported), implementations of this
method MAY return
+ * null.
*
- * @return Content
+ * @since 1.21 (may return null since 1.30)
+ *
+ * @return Content|null
*/
abstract public function makeEmptyContent();
@@ -1074,7 +1079,10 @@
}
/**
- * Return true if this content model supports direct editing, such as
via EditPage.
+ * Return true if this content model supports direct editing via
EditPage.
+ *
+ * @note ContentHandlers that return true from this method MUST return
a Content
+ * object from makeEmptyContent().
*
* @return bool Default is false, and true for TextContent and it's
derivatives.
*/
@@ -1085,6 +1093,9 @@
/**
* Whether or not this content model supports direct editing via
ApiEditPage
*
+ * @note ContentHandlers that return true from this method MUST return
a Content
+ * object from makeEmptyContent().
+ *
* @return bool Default is false, and true for TextContent and
derivatives.
*/
public function supportsDirectApiEditing() {
diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php
b/includes/jobqueue/jobs/RefreshLinksJob.php
index 02bb829..b5a9227 100644
--- a/includes/jobqueue/jobs/RefreshLinksJob.php
+++ b/includes/jobqueue/jobs/RefreshLinksJob.php
@@ -175,7 +175,8 @@
$content = $revision->getContent( Revision::RAW );
if ( !$content ) {
- // If there is no content, pretend the content is empty
+ // If there is no content, pretend the content is empty.
+ // NOTE: makeEmptyContent() can return null for some
kinds of content.
$content =
$revision->getContentHandler()->makeEmptyContent();
}
@@ -223,7 +224,7 @@
} else {
$start = microtime( true );
// Revision ID must be passed to the parser output to
get revision variables correct
- $parserOutput = $content->getParserOutput(
+ $parserOutput = $content === null ? new ParserOutput()
: $content->getParserOutput(
$title, $revision->getId(), $parserOptions,
false );
$elapsed = microtime( true ) - $start;
// If it took a long time to render, then save this
back to the cache to avoid
@@ -241,7 +242,7 @@
$stats->increment( 'refreshlinks.parser_uncached' );
}
- $updates = $content->getSecondaryDataUpdates(
+ $updates = $content === null ? [] :
$content->getSecondaryDataUpdates(
$title,
null,
!empty( $this->params['useRecursiveLinksUpdate'] ),
diff --git a/includes/specials/SpecialChangeContentModel.php
b/includes/specials/SpecialChangeContentModel.php
index a36b414..ac2d14a 100644
--- a/includes/specials/SpecialChangeContentModel.php
+++ b/includes/specials/SpecialChangeContentModel.php
@@ -198,7 +198,7 @@
$oldContent = $this->oldRevision->getContent();
try {
$newContent = ContentHandler::makeContent(
- $oldContent->getNativeData(),
$this->title, $data['model']
+ $oldContent->serialize(), $this->title,
$data['model']
);
} catch ( MWException $e ) {
return Status::newFatal(
@@ -211,7 +211,18 @@
}
} else {
// Page doesn't exist, create an empty content object
+ // NOTE: makeEmptyContent() can return null for some
kinds of content.
$newContent = ContentHandler::getForModelID(
$data['model'] )->makeEmptyContent();
+
+ if ( !$newContent ) {
+ return Status::newFatal(
+ $this->msg(
'changecontentmodel-cannot-convert' )
+ ->params(
+
$this->title->getPrefixedText(),
+
ContentHandler::getLocalizedName( $data['model'] )
+ )
+ );
+ }
}
// All other checks have passed, let's check rate limits
diff --git a/maintenance/cleanupSpam.php b/maintenance/cleanupSpam.php
index 4e47cfb..6db0a7c 100644
--- a/maintenance/cleanupSpam.php
+++ b/maintenance/cleanupSpam.php
@@ -135,7 +135,7 @@
$rev->getId()
);
} elseif ( $this->hasOption( 'delete' ) ) {
- // Didn't find a non-spammy revision, blank the
page
+ // Didn't find a non-spammy revision, delete
the page
$this->output( "deleting\n" );
$page->doDeleteArticle(
wfMessage( 'spam_deleting', $domain
)->inContentLanguage()->text()
@@ -143,13 +143,19 @@
} else {
// Didn't find a non-spammy revision, blank the
page
$handler = ContentHandler::getForTitle( $title
);
+
+ // NOTE: makeEmptyContent() can return null for
some kinds of content.
$content = $handler->makeEmptyContent();
- $this->output( "blanking\n" );
- $page->doEditContent(
- $content,
- wfMessage( 'spam_blanking', $domain
)->inContentLanguage()->text()
- );
+ if ( $content ) {
+ $this->output( "blanking\n" );
+ $page->doEditContent(
+ $content,
+ wfMessage( 'spam_blanking',
$domain )->inContentLanguage()->text()
+ );
+ } else {
+ $this->output( "can't blank!\n" );
+ }
}
$this->commitTransaction( $dbw, __METHOD__ );
}
diff --git a/tests/phpunit/structure/ContentHandlerSanityTest.php
b/tests/phpunit/structure/ContentHandlerSanityTest.php
index c8bcd60..7ee5b42 100644
--- a/tests/phpunit/structure/ContentHandlerSanityTest.php
+++ b/tests/phpunit/structure/ContentHandlerSanityTest.php
@@ -21,18 +21,25 @@
class ContentHandlerSanityTest extends MediaWikiTestCase {
- public static function provideHandlers() {
+ public static function provideMakeEmptyContent() {
$models = ContentHandler::getContentModels();
$handlers = [];
foreach ( $models as $model ) {
- $handlers[] = [ ContentHandler::getForModelID( $model )
];
+ $handler = ContentHandler::getForModelID( $model );
+
+ // Only check makeEmptyContent() for handlers that
support direct editing.
+ // Content types that do not support direct editing do
not have to support empty
+ // content, either.
+ if ( $handler->supportsDirectEditing() ||
$handler->supportsDirectApiEditing() ) {
+ $handlers[] = [ $handler ];
+ }
}
return $handlers;
}
/**
- * @dataProvider provideHandlers
+ * @dataProvider provideMakeEmptyContent
* @param ContentHandler $handler
*/
public function testMakeEmptyContent( ContentHandler $handler ) {
--
To view, visit https://gerrit.wikimedia.org/r/356228
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I028a80db8a54234ab82fd1434c0e1429d50904de
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
