Chad has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/376564 )
Change subject: Move handling for missing post content to lower level
......................................................................
Move handling for missing post content to lower level
* Don't throw for this case (even though we caught it internally).
Instead, just log the issue and return the stub.
If the caller wants to know, they can call isContentCurrentlyRetrievable.
(It is important that there's a way to distinguish, since users could
put this content manually).
* Add or tweak more debugging statements for other cases (not content,
but other places using this i18n message).
* Add public isContentCurrentlyRetrievable to check for this case
* Tweak message to 'content' instead of 'post'.
Bug: T139791
Change-Id: I2e00d13e9de347ac49843097ab1ea855773fba1d
---
M i18n/en.json
M includes/Formatter/TopicListQuery.php
M includes/Model/AbstractRevision.php
M includes/Repository/RootPostLoader.php
M includes/Templating.php
5 files changed, 34 insertions(+), 20 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Flow
refs/changes/64/376564/1
diff --git a/i18n/en.json b/i18n/en.json
index 7f7c562..889999a 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -86,7 +86,7 @@
"flow-show-change": "Show changes",
"flow-last-modified-by": "Last {{GENDER:$1|modified}} by $1",
"flow-system-usertext": "{{SITENAME}}",
- "flow-stub-post-content": "<em>Due to a technical error, this post
could not be retrieved.</em>",
+ "flow-stub-post-content": "<em>Due to a technical error, this content
could not be retrieved.</em>",
"flow-newtopic-title-placeholder": "New topic",
"flow-newtopic-content-placeholder": "Post a new message to \"$1\"",
"flow-newtopic-header": "Add a new topic",
diff --git a/includes/Formatter/TopicListQuery.php
b/includes/Formatter/TopicListQuery.php
index efd8c92..216ad59 100644
--- a/includes/Formatter/TopicListQuery.php
+++ b/includes/Formatter/TopicListQuery.php
@@ -49,6 +49,8 @@
if ( $missing ) {
$needed = [];
foreach ( $missing as $alpha ) {
+ wfDebugLog( 'Flow', __METHOD__ . ': Failed to
load latest revision for post ID ' . $alpha );
+
// convert alpha back into UUID object
$needed[] = $allPostIds[$alpha];
}
diff --git a/includes/Model/AbstractRevision.php
b/includes/Model/AbstractRevision.php
index 051fe90..082fe2b 100644
--- a/includes/Model/AbstractRevision.php
+++ b/includes/Model/AbstractRevision.php
@@ -9,6 +9,7 @@
use Flow\Conversion\Utils;
use ContentHandler;
use Hooks;
+use Sanitizer;
use Title;
use User;
use RecentChange;
@@ -353,6 +354,20 @@
}
/**
+ * Checks whether the content is retrievable.
+ *
+ * False is an error state, used when the content is unretrievable,
e.g. due to data loss (T95580)
+ * or a temporary database error.
+ *
+ * This is unrelated to whether the content is loaded on-demand.
+ *
+ * @return bool
+ */
+ public function isContentCurrentlyRetrievable() {
+ return $this->content !== false;
+ }
+
+ /**
* DO NOT USE THIS METHOD to output the content; use
* Templating::getContent, which will do additional (permissions-based)
* checks to make sure it outputs something the user can see.
@@ -362,9 +377,17 @@
* @throws InvalidDataException
*/
public function getContent( $format = 'html' ) {
- if ( $this->content === false ) {
- throw new InvalidDataException( 'Failed to load the
content' );
+ if ( !$this->isContentCurrentlyRetrievable() ) {
+ wfDebugLog( 'Flow', __METHOD__ . ': Failed to load the
content of revision with rev_id ' . $this->revId->getAlphadecimal() );
+
+ $stubContent = wfMessage( 'flow-stub-post-content'
)->parse();
+ if ( !in_array( $format, [ 'html', 'fixed-html' ] ) ) {
+ $stubContent = Sanitizer::stripAllTags(
$stubContent );
+ }
+
+ return $stubContent;
}
+
if ( $this->xssCheck === false ) {
return '';
}
diff --git a/includes/Repository/RootPostLoader.php
b/includes/Repository/RootPostLoader.php
index cfaf111..8564b30 100644
--- a/includes/Repository/RootPostLoader.php
+++ b/includes/Repository/RootPostLoader.php
@@ -146,7 +146,7 @@
$post->setReplyToId(
$parents[$postId->getAlphadecimal()] );
$posts[$postId->getAlphadecimal()] = $post;
- wfWarn( 'Missing Posts: ' . FormatJson::encode(
$missing ) );
+ wfDebugLog( 'Flow', __METHOD__ . ': Missing
posts: ' . FormatJson::encode( $missing ) );
}
}
// another helper to catch bugs in dev
diff --git a/includes/Templating.php b/includes/Templating.php
index 287d1db..8370c35 100644
--- a/includes/Templating.php
+++ b/includes/Templating.php
@@ -10,7 +10,6 @@
use Flow\Model\PostRevision;
use Flow\Parsoid\ContentFixer;
use OutputPage;
-use Sanitizer;
// These don't really belong here
use Linker;
@@ -146,21 +145,11 @@
return '';
}
- try {
- if ( $format === 'fixed-html' ) {
- // Parsoid doesn't render redlinks & doesn't
strip bad images
- $content = $this->contentFixer->getContent(
$revision );
- } else {
- $content = $revision->getContent( $format );
- }
- } catch ( \Exception $e ) {
- wfDebugLog( 'Flow', __METHOD__ . ': Failed to get
content for rev_id = ' . $revision->getRevisionId()->getAlphadecimal() );
- \MWExceptionHandler::logException( $e );
-
- $content = wfMessage( 'flow-stub-post-content'
)->parse();
- if ( !in_array( $format, [ 'html', 'fixed-html' ] ) ) {
- $content = Sanitizer::stripAllTags( $content );
- }
+ if ( $format === 'fixed-html' ) {
+ // Parsoid doesn't render redlinks & doesn't strip bad
images
+ $content = $this->contentFixer->getContent( $revision );
+ } else {
+ $content = $revision->getContent( $format );
}
return $content;
--
To view, visit https://gerrit.wikimedia.org/r/376564
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e00d13e9de347ac49843097ab1ea855773fba1d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Flow
Gerrit-Branch: wmf/1.30.0-wmf.17
Gerrit-Owner: Chad <ch...@wikimedia.org>
Gerrit-Reviewer: Mattflaschen <mflasc...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
