From 880f9755e05c369fc865ad28fcecf147149d13ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartosz=20Dziewo=C5=84ski?= Date: Fri, 18 Mar 2022 04:28:06 +0100 Subject: [PATCH] Separate ContentThreadItem and DatabaseThreadItem etc. Rename ThreadItem to ContentThreadItem, then create a new ThreadItem interface containing only the methods that we'll be able to implement using only the persistently stored data (no parsing), then create a DatabaseThreadItem. Do the same for CommentItem and HeadingItem. ThreadItemSet gets a similar treatment, but it's basically only for Phan's type checking. (This is sad.) Change-Id: I1633049befe8ec169753b82eb876459af1f63fe8 --- includes/ApiDiscussionToolsEdit.php | 5 +- includes/ApiDiscussionToolsPageInfo.php | 22 ++-- includes/ApiDiscussionToolsTrait.php | 4 +- includes/CommentFormatter.php | 16 +-- includes/CommentModifier.php | 26 +++-- includes/CommentParser.php | 65 ++++++----- includes/CommentUtils.php | 14 ++- includes/ContentThreadItemSet.php | 108 ++++++++++++++++++ includes/DatabaseThreadItemSet.php | 103 +++++++++++++++++ includes/Notifications/EventDispatcher.php | 25 ++-- includes/ThreadItem/CommentItem.php | 22 ++++ includes/ThreadItem/CommentItemTrait.php | 94 +++++++++++++++ .../ContentCommentItem.php} | 88 +++----------- .../ContentHeadingItem.php} | 41 ++----- .../ContentThreadItem.php} | 48 +++----- includes/ThreadItem/DatabaseCommentItem.php | 64 +++++++++++ includes/ThreadItem/DatabaseHeadingItem.php | 46 ++++++++ includes/ThreadItem/DatabaseThreadItem.php | 101 ++++++++++++++++ includes/ThreadItem/HeadingItem.php | 15 +++ includes/ThreadItem/HeadingItemTrait.php | 44 +++++++ includes/ThreadItem/ThreadItem.php | 53 +++++++++ includes/ThreadItem/ThreadItemTrait.php | 38 ++++++ includes/ThreadItemSet.php | 69 +++-------- tests/phpunit/CommentParserTest.php | 16 +-- ...ItemTest.php => ContentThreadItemTest.php} | 25 ++-- tests/phpunit/MockEventDispatcher.php | 10 +- 26 files changed, 868 insertions(+), 294 deletions(-) create mode 100644 includes/ContentThreadItemSet.php create mode 100644 includes/DatabaseThreadItemSet.php create mode 100644 includes/ThreadItem/CommentItem.php create mode 100644 includes/ThreadItem/CommentItemTrait.php rename includes/{CommentItem.php => ThreadItem/ContentCommentItem.php} (66%) rename includes/{HeadingItem.php => ThreadItem/ContentHeadingItem.php} (68%) rename includes/{ThreadItem.php => ThreadItem/ContentThreadItem.php} (90%) create mode 100644 includes/ThreadItem/DatabaseCommentItem.php create mode 100644 includes/ThreadItem/DatabaseHeadingItem.php create mode 100644 includes/ThreadItem/DatabaseThreadItem.php create mode 100644 includes/ThreadItem/HeadingItem.php create mode 100644 includes/ThreadItem/HeadingItemTrait.php create mode 100644 includes/ThreadItem/ThreadItem.php create mode 100644 includes/ThreadItem/ThreadItemTrait.php rename tests/phpunit/{ThreadItemTest.php => ContentThreadItemTest.php} (85%) diff --git a/includes/ApiDiscussionToolsEdit.php b/includes/ApiDiscussionToolsEdit.php index 64199646b..6d3bb7339 100644 --- a/includes/ApiDiscussionToolsEdit.php +++ b/includes/ApiDiscussionToolsEdit.php @@ -7,6 +7,7 @@ use ApiMain; use DerivativeContext; use DerivativeRequest; use MediaWiki\Extension\DiscussionTools\Hooks\HookUtils; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; use MediaWiki\Extension\VisualEditor\ApiParsoidTrait; use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; @@ -232,7 +233,7 @@ class ApiDiscussionToolsEdit extends ApiBase { if ( $commentId ) { $comment = $threadItemSet->findCommentById( $commentId ); - if ( !$comment || !( $comment instanceof CommentItem ) ) { + if ( !$comment || !( $comment instanceof ContentCommentItem ) ) { $this->dieWithError( [ 'apierror-discussiontools-commentid-notfound', $commentId ] ); } @@ -242,7 +243,7 @@ class ApiDiscussionToolsEdit extends ApiBase { if ( count( $comments ) > 1 ) { $this->dieWithError( [ 'apierror-discussiontools-commentname-ambiguous', $commentName ] ); - } elseif ( !$comment || !( $comment instanceof CommentItem ) ) { + } elseif ( !$comment || !( $comment instanceof ContentCommentItem ) ) { $this->dieWithError( [ 'apierror-discussiontools-commentname-notfound', $commentName ] ); } } diff --git a/includes/ApiDiscussionToolsPageInfo.php b/includes/ApiDiscussionToolsPageInfo.php index cf7251680..a702d2e26 100644 --- a/includes/ApiDiscussionToolsPageInfo.php +++ b/includes/ApiDiscussionToolsPageInfo.php @@ -4,6 +4,8 @@ namespace MediaWiki\Extension\DiscussionTools; use ApiBase; use ApiMain; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentHeadingItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem; use MediaWiki\Extension\VisualEditor\ApiParsoidTrait; use Title; use Wikimedia\ParamValidator\ParamValidator; @@ -50,12 +52,12 @@ class ApiDiscussionToolsPageInfo extends ApiBase { } /** - * Get transcluded=from data for a ThreadItemSet + * Get transcluded=from data for a ContentThreadItemSet * - * @param ThreadItemSet $threadItemSet + * @param ContentThreadItemSet $threadItemSet * @return array */ - private static function getTranscludedFrom( ThreadItemSet $threadItemSet ): array { + private static function getTranscludedFrom( ContentThreadItemSet $threadItemSet ): array { $threadItems = $threadItemSet->getThreadItems(); $transcludedFrom = []; foreach ( $threadItems as $threadItem ) { @@ -84,33 +86,33 @@ class ApiDiscussionToolsPageInfo extends ApiBase { } /** - * Get thread items HTML for a ThreadItemSet + * Get thread items HTML for a ContentThreadItemSet * - * @param ThreadItemSet $threadItemSet + * @param ContentThreadItemSet $threadItemSet * @return array */ - private static function getThreadItemsHtml( ThreadItemSet $threadItemSet ): array { + private static function getThreadItemsHtml( ContentThreadItemSet $threadItemSet ): array { $threads = $threadItemSet->getThreads(); if ( count( $threads ) > 0 ) { $firstHeading = $threads[0]; if ( !$firstHeading->isPlaceholderHeading() ) { $range = new ImmutableRange( $firstHeading->getRootNode(), 0, $firstHeading->getRootNode(), 0 ); - $fakeHeading = new HeadingItem( $range, null ); + $fakeHeading = new ContentHeadingItem( $range, null ); $fakeHeading->setRootNode( $firstHeading->getRootNode() ); $fakeHeading->setName( 'h-' ); $fakeHeading->setId( 'h-' ); array_unshift( $threads, $fakeHeading ); } } - $output = array_map( static function ( ThreadItem $item ) { - return $item->jsonSerialize( true, static function ( array &$array, ThreadItem $item ) { + $output = array_map( static function ( ContentThreadItem $item ) { + return $item->jsonSerialize( true, static function ( array &$array, ContentThreadItem $item ) { $array['html'] = $item->getHtml(); } ); }, $threads ); foreach ( $threads as $index => $item ) { // need to loop over this to fix up empty sections, because we // need context that's not available inside the array map - if ( $item instanceof HeadingItem && count( $item->getReplies() ) === 0 ) { + if ( $item instanceof ContentHeadingItem && count( $item->getReplies() ) === 0 ) { $nextItem = $threads[ $index + 1 ] ?? false; $startRange = $item->getRange(); if ( $nextItem ) { diff --git a/includes/ApiDiscussionToolsTrait.php b/includes/ApiDiscussionToolsTrait.php index 5402d0d0d..71c251de1 100644 --- a/includes/ApiDiscussionToolsTrait.php +++ b/includes/ApiDiscussionToolsTrait.php @@ -20,9 +20,9 @@ use Wikimedia\Parsoid\Utils\DOMUtils; trait ApiDiscussionToolsTrait { /** * @param RevisionRecord $revision - * @return ThreadItemSet + * @return ContentThreadItemSet */ - protected function parseRevision( RevisionRecord $revision ): ThreadItemSet { + protected function parseRevision( RevisionRecord $revision ): ContentThreadItemSet { $response = $this->requestRestbasePageHtml( $revision ); $doc = DOMUtils::parseHTML( $response['body'] ); diff --git a/includes/CommentFormatter.php b/includes/CommentFormatter.php index 342c81c6f..2a45a10f8 100644 --- a/includes/CommentFormatter.php +++ b/includes/CommentFormatter.php @@ -5,6 +5,8 @@ namespace MediaWiki\Extension\DiscussionTools; use Html; use Language; use MediaWiki\Extension\DiscussionTools\Hooks\HookUtils; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentHeadingItem; use MediaWiki\MediaWikiServices; use MediaWiki\User\UserIdentity; use MWExceptionHandler; @@ -81,9 +83,9 @@ class CommentFormatter { * Add a topic container around a heading element * * @param Element $headingElement Heading element - * @param HeadingItem|null $headingItem Heading item + * @param ContentHeadingItem|null $headingItem Heading item */ - protected static function addTopicContainer( Element $headingElement, ?HeadingItem $headingItem = null ) { + protected static function addTopicContainer( Element $headingElement, ?ContentHeadingItem $headingItem = null ) { $doc = $headingElement->ownerDocument; DOMCompat::getClassList( $headingElement )->add( 'ext-discussiontools-init-section' ); @@ -182,7 +184,7 @@ class CommentFormatter { foreach ( array_reverse( $threadItems ) as $threadItem ) { // TODO: Consider not attaching JSON data to the DOM. // Create a dummy node to attach data to. - if ( $threadItem instanceof HeadingItem && $threadItem->isPlaceholderHeading() ) { + if ( $threadItem instanceof ContentHeadingItem && $threadItem->isPlaceholderHeading() ) { $node = $doc->createElement( 'span' ); $container->insertBefore( $node, $container->firstChild ); $threadItem->setRange( new ImmutableRange( $node, 0, $node, 0 ) ); @@ -218,7 +220,7 @@ class CommentFormatter { $itemData = $threadItem->jsonSerialize(); $itemJSON = json_encode( $itemData ); - if ( $threadItem instanceof HeadingItem ) { + if ( $threadItem instanceof ContentHeadingItem ) { // , or in Parsoid HTML $headline = $threadItem->getRange()->endContainer; Assert::precondition( $headline instanceof Element, 'HeadingItem refers to an element node' ); @@ -230,7 +232,7 @@ class CommentFormatter { static::addTopicContainer( $headingElement, $threadItem ); } } - } elseif ( $threadItem instanceof CommentItem ) { + } elseif ( $threadItem instanceof ContentCommentItem ) { $replyLinkButtons = $doc->createElement( 'span' ); $replyLinkButtons->setAttribute( 'class', 'ext-discussiontools-init-replylink-buttons' ); @@ -438,10 +440,10 @@ class CommentFormatter { /** * Get JSON for a commentItem that can be inserted into a comment marker * - * @param CommentItem $commentItem Comment item + * @param ContentCommentItem $commentItem Comment item * @return string */ - private static function getJsonForCommentMarker( CommentItem $commentItem ): string { + private static function getJsonForCommentMarker( ContentCommentItem $commentItem ): string { $JSON = [ 'id' => $commentItem->getId(), 'timestamp' => $commentItem->getTimestampString() diff --git a/includes/CommentModifier.php b/includes/CommentModifier.php index bb4ad96e8..f07443096 100644 --- a/includes/CommentModifier.php +++ b/includes/CommentModifier.php @@ -2,6 +2,8 @@ namespace MediaWiki\Extension\DiscussionTools; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem; use MediaWiki\MediaWikiServices; use MWException; use Wikimedia\Assert\Assert; @@ -47,10 +49,10 @@ class CommentModifier { * Given a comment and a reply link, add the reply link to its document's DOM tree, at the end of * the comment. * - * @param CommentItem $comment + * @param ContentCommentItem $comment * @param Element $linkNode Reply link */ - public static function addReplyLink( CommentItem $comment, Element $linkNode ): void { + public static function addReplyLink( ContentCommentItem $comment, Element $linkNode ): void { $target = $comment->getRange()->endContainer; // Insert the link before trailing whitespace. @@ -77,13 +79,13 @@ class CommentModifier { * The DOM tree is suitably rearranged to ensure correct indentation level of the reply (wrapper * nodes are added, and other nodes may be moved around). * - * @param ThreadItem $comment + * @param ContentThreadItem $comment * @param string $replyIndentation Reply indentation syntax to use, one of: * - 'invisible' (use `
` tags to output `:` in wikitext) * - 'bullet' (use `
  • ` tags to output `*` in wikitext) * @return Element */ - public static function addListItem( ThreadItem $comment, string $replyIndentation ): Element { + public static function addListItem( ContentThreadItem $comment, string $replyIndentation ): Element { $listTypeMap = [ 'li' => 'ul', 'dd' => 'dl' @@ -466,10 +468,10 @@ class CommentModifier { /** * Add a reply to a specific comment * - * @param ThreadItem $comment Comment being replied to + * @param ContentThreadItem $comment Comment being replied to * @param DocumentFragment $container Container of comment DOM nodes */ - public static function addReply( ThreadItem $comment, DocumentFragment $container ): void { + public static function addReply( ContentThreadItem $comment, DocumentFragment $container ): void { $services = MediaWikiServices::getInstance(); $dtConfig = $services->getConfigFactory()->makeConfig( 'discussiontools' ); $replyIndentation = $dtConfig->get( 'DiscussionToolsReplyIndentation' ); @@ -573,11 +575,13 @@ class CommentModifier { /** * Add a reply in the DOM to a comment using wikitext. * - * @param CommentItem $comment Comment being replied to + * @param ContentCommentItem $comment Comment being replied to * @param string $wikitext * @param string|null $signature */ - public static function addWikitextReply( CommentItem $comment, string $wikitext, string $signature = null ): void { + public static function addWikitextReply( + ContentCommentItem $comment, string $wikitext, string $signature = null + ): void { $doc = $comment->getRange()->endContainer->ownerDocument; $container = static::prepareWikitextReply( $doc, $wikitext ); if ( $signature !== null ) { @@ -589,11 +593,13 @@ class CommentModifier { /** * Add a reply in the DOM to a comment using HTML. * - * @param CommentItem $comment Comment being replied to + * @param ContentCommentItem $comment Comment being replied to * @param string $html * @param string|null $signature */ - public static function addHtmlReply( CommentItem $comment, string $html, string $signature = null ): void { + public static function addHtmlReply( + ContentCommentItem $comment, string $html, string $signature = null + ): void { $doc = $comment->getRange()->endContainer->ownerDocument; $container = static::prepareHtmlReply( $doc, $html ); if ( $signature !== null ) { diff --git a/includes/CommentParser.php b/includes/CommentParser.php index cdc506809..f9b136ebb 100644 --- a/includes/CommentParser.php +++ b/includes/CommentParser.php @@ -9,6 +9,11 @@ use DateTimeImmutable; use DateTimeZone; use Language; use MalformedTitleException; +use MediaWiki\Extension\DiscussionTools\ThreadItem\CommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentHeadingItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ThreadItem; use MediaWiki\Languages\LanguageConverterFactory; use MWException; use TitleParser; @@ -84,9 +89,9 @@ class CommentParser { * * @param Element $rootNode Root node of content to parse * @param TitleValue $title Title of the page being parsed - * @return ThreadItemSet + * @return ContentThreadItemSet */ - public function parse( Element $rootNode, TitleValue $title ): ThreadItemSet { + public function parse( Element $rootNode, TitleValue $title ): ContentThreadItemSet { $this->rootNode = $rootNode; $this->title = $title; @@ -784,8 +789,8 @@ class CommentParser { return $sigRange; } - private function buildThreadItems(): ThreadItemSet { - $result = new ThreadItemSet(); + private function buildThreadItems(): ContentThreadItemSet { + $result = new ContentThreadItemSet(); $timestampRegexps = $this->getLocalTimestampRegexps(); $dfParsers = $this->getLocalTimestampParsers(); @@ -806,7 +811,7 @@ class CommentParser { $range = new ImmutableRange( $headingNode, $startOffset, $headingNode, $headingNode->childNodes->length ); - $curComment = new HeadingItem( $range, (int)( $match[ 1 ] ) ); + $curComment = new ContentHeadingItem( $range, (int)( $match[ 1 ] ) ); $curComment->setRootNode( $this->rootNode ); $result->addThreadItem( $curComment ); $curCommentEnd = $node; @@ -896,7 +901,7 @@ class CommentParser { $warnings[] = $dateWarning; } - $curComment = new CommentItem( + $curComment = new ContentCommentItem( $level, $range, $sigRanges, @@ -911,7 +916,7 @@ class CommentParser { // Add a fake placeholder heading if there are any comments in the 0th section // (before the first real heading) $range = new ImmutableRange( $this->rootNode, 0, $this->rootNode, 0 ); - $fakeHeading = new HeadingItem( $range, null ); + $fakeHeading = new ContentHeadingItem( $range, null ); $fakeHeading->setRootNode( $this->rootNode ); $result->addThreadItem( $fakeHeading ); } @@ -936,25 +941,25 @@ class CommentParser { /** * Given a thread item, return an identifier for it that is unique within the page. * - * @param ThreadItem $threadItem - * @param ThreadItemSet $previousItems + * @param ContentThreadItem $threadItem + * @param ContentThreadItemSet $previousItems * @return string */ - private function computeId( ThreadItem $threadItem, ThreadItemSet $previousItems ): string { + private function computeId( ContentThreadItem $threadItem, ContentThreadItemSet $previousItems ): string { // When changing the algorithm below, copy the old version into computeLegacyId() // for compatibility with cached data. $id = null; - if ( $threadItem instanceof HeadingItem && $threadItem->isPlaceholderHeading() ) { + if ( $threadItem instanceof ContentHeadingItem && $threadItem->isPlaceholderHeading() ) { // The range points to the root note, using it like below results in silly values $id = 'h-'; - } elseif ( $threadItem instanceof HeadingItem ) { + } elseif ( $threadItem instanceof ContentHeadingItem ) { // , or in Parsoid HTML $headline = $threadItem->getRange()->startContainer; Assert::precondition( $headline instanceof Element, 'HeadingItem refers to an element node' ); $id = 'h-' . $this->truncateForId( $headline->getAttribute( 'id' ) ?? '' ); - } elseif ( $threadItem instanceof CommentItem ) { + } elseif ( $threadItem instanceof ContentCommentItem ) { $id = 'c-' . $this->truncateForId( str_replace( ' ', '_', $threadItem->getAuthor() ) ) . '-' . $threadItem->getTimestampString(); } else { @@ -964,17 +969,17 @@ class CommentParser { // If there would be multiple comments with the same ID (i.e. the user left multiple comments // in one edit, or within a minute), add the parent ID to disambiguate them. $threadItemParent = $threadItem->getParent(); - if ( $threadItemParent instanceof HeadingItem && !$threadItemParent->isPlaceholderHeading() ) { + if ( $threadItemParent instanceof ContentHeadingItem && !$threadItemParent->isPlaceholderHeading() ) { // , or in Parsoid HTML $headline = $threadItemParent->getRange()->startContainer; Assert::precondition( $headline instanceof Element, 'HeadingItem refers to an element node' ); $id .= '-' . $this->truncateForId( $headline->getAttribute( 'id' ) ?? '' ); - } elseif ( $threadItemParent instanceof CommentItem ) { + } elseif ( $threadItemParent instanceof ContentCommentItem ) { $id .= '-' . $this->truncateForId( str_replace( ' ', '_', $threadItemParent->getAuthor() ) ) . '-' . $threadItemParent->getTimestampString(); } - if ( $threadItem instanceof HeadingItem ) { + if ( $threadItem instanceof ContentHeadingItem ) { // To avoid old threads re-appearing on popular pages when someone uses a vague title // (e.g. dozens of threads titled "question" on [[Wikipedia:Help desk]]: https://w.wiki/fbN), // include the oldest timestamp in the thread (i.e. date the thread was started) in the @@ -1003,11 +1008,11 @@ class CommentParser { * Given a thread item, return an identifier for it like computeId(), generated according to an * older algorithm, so that we can still match IDs from cached data. * - * @param ThreadItem $threadItem - * @param ThreadItemSet $previousItems + * @param ContentThreadItem $threadItem + * @param ContentThreadItemSet $previousItems * @return string|null */ - private function computeLegacyId( ThreadItem $threadItem, ThreadItemSet $previousItems ): ?string { + private function computeLegacyId( ContentThreadItem $threadItem, ContentThreadItemSet $previousItems ): ?string { // When we change the algorithm in computeId(), the old version should be copied below // for compatibility with cached data. @@ -1021,16 +1026,16 @@ class CommentParser { * * Multiple comments on a page can have the same name; use ID to distinguish them. * - * @param ThreadItem $threadItem + * @param ContentThreadItem $threadItem * @return string */ - private function computeName( ThreadItem $threadItem ): string { + private function computeName( ContentThreadItem $threadItem ): string { $name = null; - if ( $threadItem instanceof HeadingItem ) { + if ( $threadItem instanceof ContentHeadingItem ) { $name = 'h-'; $mainComment = $this->getThreadStartComment( $threadItem ); - } elseif ( $threadItem instanceof CommentItem ) { + } elseif ( $threadItem instanceof ContentCommentItem ) { $name = 'c-'; $mainComment = $threadItem; } else { @@ -1046,9 +1051,9 @@ class CommentParser { } /** - * @param ThreadItemSet $result + * @param ContentThreadItemSet $result */ - private function buildThreads( ThreadItemSet $result ): void { + private function buildThreads( ContentThreadItemSet $result ): void { $lastHeading = null; $replies = []; @@ -1062,7 +1067,7 @@ class CommentParser { } } - if ( $threadItem instanceof HeadingItem ) { + if ( $threadItem instanceof ContentHeadingItem ) { // New root (thread) // Attach as a sub-thread to preceding higher-level heading. // Any replies will appear in the tree twice, under the main-thread and the sub-thread. @@ -1094,9 +1099,9 @@ class CommentParser { * This has to be a separate pass because we don't have the list of replies before * this point. * - * @param ThreadItemSet $result + * @param ContentThreadItemSet $result */ - private function computeIdsAndNames( ThreadItemSet $result ): void { + private function computeIdsAndNames( ContentThreadItemSet $result ): void { foreach ( $result->getThreadItems() as $threadItem ) { $name = $this->computeName( $threadItem ); $threadItem->setName( $name ); @@ -1116,14 +1121,14 @@ class CommentParser { */ private function getThreadStartComment( ThreadItem $threadItem ): ?CommentItem { $oldest = null; - if ( $threadItem instanceof CommentItem ) { + if ( $threadItem instanceof ContentCommentItem ) { $oldest = $threadItem; } // Check all replies. This can't just use the first comment because threads are often summarized // at the top when the discussion is closed. foreach ( $threadItem->getReplies() as $comment ) { // Don't include sub-threads to avoid changing the ID when threads are "merged". - if ( $comment instanceof CommentItem ) { + if ( $comment instanceof ContentCommentItem ) { $oldestInReplies = $this->getThreadStartComment( $comment ); if ( !$oldest || $oldestInReplies->getTimestamp() < $oldest->getTimestamp() ) { $oldest = $oldestInReplies; diff --git a/includes/CommentUtils.php b/includes/CommentUtils.php index 2ec04c2e6..4d10257b2 100644 --- a/includes/CommentUtils.php +++ b/includes/CommentUtils.php @@ -4,6 +4,8 @@ namespace MediaWiki\Extension\DiscussionTools; use Config; use LogicException; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem; use Wikimedia\Assert\Assert; use Wikimedia\Parsoid\DOM\Comment; use Wikimedia\Parsoid\DOM\Element; @@ -389,13 +391,15 @@ class CommentUtils { /** * Get the nodes (if any) that contain the given thread item, and nothing else. * - * @param ThreadItem $item + * @param ContentThreadItem $item * @param ?Node $excludedAncestorNode Node that shouldn't be included in the result, even if it * contains the item and nothing else. This is intended to avoid traversing outside of a node * which is a container for all the thread items. * @return Node[]|null */ - public static function getFullyCoveredSiblings( ThreadItem $item, ?Node $excludedAncestorNode = null ): ?array { + public static function getFullyCoveredSiblings( + ContentThreadItem $item, ?Node $excludedAncestorNode = null + ): ?array { $siblings = static::getCoveredSiblings( $item->getRange() ); $makeRange = static function ( $siblings ) { @@ -710,13 +714,13 @@ class CommentUtils { * signature, or there's some text within the same paragraph that was detected as part of the same * comment). * - * @param ThreadItemSet $itemSet + * @param ContentThreadItemSet $itemSet * @param string $author * @param Element $rootNode * @return bool */ public static function isSingleCommentSignedBy( - ThreadItemSet $itemSet, + ContentThreadItemSet $itemSet, string $author, Element $rootNode ): bool { @@ -725,7 +729,7 @@ class CommentUtils { if ( $items ) { $lastItem = end( $items ); // Check that we've detected a comment first, not just headings (T304377) - if ( !( $lastItem instanceof CommentItem && $lastItem->getAuthor() === $author ) ) { + if ( !( $lastItem instanceof ContentCommentItem && $lastItem->getAuthor() === $author ) ) { return false; } diff --git a/includes/ContentThreadItemSet.php b/includes/ContentThreadItemSet.php new file mode 100644 index 000000000..681e3d86b --- /dev/null +++ b/includes/ContentThreadItemSet.php @@ -0,0 +1,108 @@ +threadItems[] = $item; + if ( $item instanceof CommentItem ) { + $this->commentItems[] = $item; + } + if ( $item instanceof HeadingItem ) { + $this->threads[] = $item; + } + } + + /** + * @inheritDoc + */ + public function isEmpty(): bool { + return !$this->threadItems; + } + + /** + * @inheritDoc + * @param ThreadItem $item + */ + public function updateIdAndNameMaps( ThreadItem $item ) { + Assert::precondition( $item instanceof ContentThreadItem, 'Must be ContentThreadItem' ); + + $this->threadItemsByName[ $item->getName() ][] = $item; + + $this->threadItemsById[ $item->getId() ] = $item; + + $legacyId = $item->getLegacyId(); + if ( $legacyId ) { + $this->threadItemsById[ $legacyId ] = $item; + } + } + + /** + * @inheritDoc + * @return ContentThreadItem[] Thread items + */ + public function getThreadItems(): array { + return $this->threadItems; + } + + /** + * @inheritDoc + * @return ContentCommentItem[] Comment items + */ + public function getCommentItems(): array { + return $this->commentItems; + } + + /** + * @inheritDoc + * @return ContentThreadItem[] Thread items, empty array if not found + */ + public function findCommentsByName( string $name ): array { + return $this->threadItemsByName[$name] ?? []; + } + + /** + * @inheritDoc + * @return ContentThreadItem|null Thread item, null if not found + */ + public function findCommentById( string $id ): ?ThreadItem { + return $this->threadItemsById[$id] ?? null; + } + + /** + * @inheritDoc + * @return ContentHeadingItem[] Tree structure of comments, top-level items are the headings. + */ + public function getThreads(): array { + return $this->threads; + } +} diff --git a/includes/DatabaseThreadItemSet.php b/includes/DatabaseThreadItemSet.php new file mode 100644 index 000000000..496b82fd8 --- /dev/null +++ b/includes/DatabaseThreadItemSet.php @@ -0,0 +1,103 @@ +threadItems[] = $item; + if ( $item instanceof CommentItem ) { + $this->commentItems[] = $item; + } + if ( $item instanceof HeadingItem ) { + $this->threads[] = $item; + } + } + + /** + * @inheritDoc + */ + public function isEmpty(): bool { + return !$this->threadItems; + } + + /** + * @inheritDoc + * @param ThreadItem $item + */ + public function updateIdAndNameMaps( ThreadItem $item ) { + Assert::precondition( $item instanceof DatabaseThreadItem, 'Must be DatabaseThreadItem' ); + + $this->threadItemsByName[ $item->getName() ][] = $item; + + $this->threadItemsById[ $item->getId() ] = $item; + } + + /** + * @inheritDoc + * @return DatabaseThreadItem[] Thread items + */ + public function getThreadItems(): array { + return $this->threadItems; + } + + /** + * @inheritDoc + * @return DatabaseCommentItem[] Comment items + */ + public function getCommentItems(): array { + return $this->commentItems; + } + + /** + * @inheritDoc + * @return DatabaseThreadItem[] Thread items, empty array if not found + */ + public function findCommentsByName( string $name ): array { + return $this->threadItemsByName[$name] ?? []; + } + + /** + * @inheritDoc + * @return DatabaseThreadItem|null Thread item, null if not found + */ + public function findCommentById( string $id ): ?ThreadItem { + return $this->threadItemsById[$id] ?? null; + } + + /** + * @inheritDoc + * @return DatabaseHeadingItem[] Tree structure of comments, top-level items are the headings. + */ + public function getThreads(): array { + return $this->threads; + } +} diff --git a/includes/Notifications/EventDispatcher.php b/includes/Notifications/EventDispatcher.php index aec4de2bd..76c616590 100644 --- a/includes/Notifications/EventDispatcher.php +++ b/includes/Notifications/EventDispatcher.php @@ -18,13 +18,14 @@ use Error; use ExtensionRegistry; use IDBAccessObject; use Iterator; -use MediaWiki\Extension\DiscussionTools\CommentItem; -use MediaWiki\Extension\DiscussionTools\HeadingItem; +use MediaWiki\Extension\DiscussionTools\ContentThreadItemSet; use MediaWiki\Extension\DiscussionTools\Hooks\HookUtils; use MediaWiki\Extension\DiscussionTools\SubscriptionItem; use MediaWiki\Extension\DiscussionTools\SubscriptionStore; -use MediaWiki\Extension\DiscussionTools\ThreadItem; -use MediaWiki\Extension\DiscussionTools\ThreadItemSet; +use MediaWiki\Extension\DiscussionTools\ThreadItem\CommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\HeadingItem; use MediaWiki\Extension\EventLogging\EventLogging; use MediaWiki\MediaWikiServices; use MediaWiki\Page\PageIdentity; @@ -41,9 +42,9 @@ use Wikimedia\Parsoid\Utils\DOMUtils; class EventDispatcher { /** * @param RevisionRecord $revRecord - * @return ThreadItemSet + * @return ContentThreadItemSet */ - private static function getParsedRevision( RevisionRecord $revRecord ): ThreadItemSet { + private static function getParsedRevision( RevisionRecord $revRecord ): ContentThreadItemSet { $services = MediaWikiServices::getInstance(); $pageRecord = $services->getPageStore()->getPageById( $revRecord->getPageId() ) ?: @@ -121,8 +122,8 @@ class EventDispatcher { * For any other headings (including level 3+ before the first level 2 heading, level 1, and * section zero placeholder headings), ignore comments in those threads. * - * @param ThreadItem[] $items - * @return CommentItem[][][] + * @param ContentThreadItem[] $items + * @return ContentCommentItem[][][] */ private static function groupCommentsByThreadAndName( array $items ): array { $comments = []; @@ -143,16 +144,16 @@ class EventDispatcher { * Helper for generateEventsForRevision(), separated out for easier testing. * * @param array &$events - * @param ThreadItemSet $oldItemSet - * @param ThreadItemSet $newItemSet + * @param ContentThreadItemSet $oldItemSet + * @param ContentThreadItemSet $newItemSet * @param RevisionRecord $newRevRecord * @param PageIdentity $title * @param UserIdentity $user */ protected static function generateEventsFromItemSets( array &$events, - ThreadItemSet $oldItemSet, - ThreadItemSet $newItemSet, + ContentThreadItemSet $oldItemSet, + ContentThreadItemSet $newItemSet, RevisionRecord $newRevRecord, PageIdentity $title, UserIdentity $user diff --git a/includes/ThreadItem/CommentItem.php b/includes/ThreadItem/CommentItem.php new file mode 100644 index 000000000..ab60e650a --- /dev/null +++ b/includes/ThreadItem/CommentItem.php @@ -0,0 +1,22 @@ + $this->getTimestampString(), + 'author' => $this->getAuthor(), + ] ); + } + + /** + * @return array JSON-serializable array + */ + public function jsonSerializeForDiff(): array { + $data = $this->jsonSerialize(); + + $heading = $this->getHeading(); + $data['headingId'] = $heading->getId(); + $subscribableHeading = $this->getSubscribableHeading(); + $data['subscribableHeadingId'] = $subscribableHeading ? $subscribableHeading->getId() : null; + + return $data; + } + + /** + * Get the comment timestamp in the format used in IDs and names. + * + * Depending on the date of the comment, this may use one of two formats: + * + * - For dates prior to 'DiscussionToolsTimestampFormatSwitchTime' (by default 2022-07-12): + * Uses ISO 8601 date. Almost DateTimeInterface::RFC3339_EXTENDED, but ending with 'Z' instead + * of '+00:00', like Date#toISOString in JavaScript. + * + * - For dates on or after 'DiscussionToolsTimestampFormatSwitchTime' (by default 2022-07-12): + * Uses MediaWiki timestamp (TS_MW in MediaWiki PHP code). + * + * @return string Comment timestamp in standard format + */ + public function getTimestampString(): string { + $dtConfig = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'discussiontools' ); + $switchTime = new DateTimeImmutable( + $dtConfig->get( 'DiscussionToolsTimestampFormatSwitchTime' ) + ); + $timestamp = $this->getTimestamp(); + if ( $timestamp < $switchTime ) { + return $timestamp->format( 'Y-m-d\TH:i:s.v\Z' ); + } else { + return $timestamp->format( 'YmdHis' ); + } + } + + /** + * @return ContentHeadingItem Closest ancestor which is a HeadingItem + */ + public function getHeading(): HeadingItem { + $parent = $this; + while ( $parent instanceof CommentItem ) { + $parent = $parent->getParent(); + } + if ( !( $parent instanceof HeadingItem ) ) { + throw new MWException( 'heading parent not found' ); + } + return $parent; + } + + /** + * @return ContentHeadingItem|null Closest heading that can be used for topic subscriptions + */ + public function getSubscribableHeading(): ?HeadingItem { + $heading = $this->getHeading(); + while ( $heading instanceof HeadingItem && !$heading->isSubscribable() ) { + $heading = $heading->getParent(); + } + return $heading instanceof HeadingItem ? $heading : null; + } +} diff --git a/includes/CommentItem.php b/includes/ThreadItem/ContentCommentItem.php similarity index 66% rename from includes/CommentItem.php rename to includes/ThreadItem/ContentCommentItem.php index 51a7835fb..a30c6aa5b 100644 --- a/includes/CommentItem.php +++ b/includes/ThreadItem/ContentCommentItem.php @@ -1,10 +1,12 @@ author = $author; } - /** - * @inheritDoc - */ - public function jsonSerialize( bool $deep = false, ?callable $callback = null ): array { - return array_merge( parent::jsonSerialize( $deep, $callback ), [ - 'timestamp' => $this->getTimestampString(), - 'author' => $this->author, - ] ); - } - - /** - * @return array JSON-serializable array - */ - public function jsonSerializeForDiff(): array { - $data = $this->jsonSerialize(); - - $heading = $this->getHeading(); - $data['headingId'] = $heading->getId(); - $subscribableHeading = $this->getSubscribableHeading(); - $data['subscribableHeadingId'] = $subscribableHeading ? $subscribableHeading->getId() : null; - - return $data; - } - /** * Get the HTML of this comment's body * @@ -163,33 +146,6 @@ class CommentItem extends ThreadItem { return $this->timestamp; } - /** - * Get the comment timestamp in the format used in IDs and names. - * - * Depending on the date of the comment, this may use one of two formats: - * - * - For dates prior to 'DiscussionToolsTimestampFormatSwitchTime' (by default 2022-07-12): - * Uses ISO 8601 date. Almost DateTimeInterface::RFC3339_EXTENDED, but ending with 'Z' instead - * of '+00:00', like Date#toISOString in JavaScript. - * - * - For dates on or after 'DiscussionToolsTimestampFormatSwitchTime' (by default 2022-07-12): - * Uses MediaWiki timestamp (TS_MW in MediaWiki PHP code). - * - * @return string Comment timestamp in standard format - */ - public function getTimestampString(): string { - $dtConfig = MediaWikiServices::getInstance()->getConfigFactory()->makeConfig( 'discussiontools' ); - $switchTime = new DateTimeImmutable( - $dtConfig->get( 'DiscussionToolsTimestampFormatSwitchTime' ) - ); - $timestamp = $this->getTimestamp(); - if ( $timestamp < $switchTime ) { - return $timestamp->format( 'Y-m-d\TH:i:s.v\Z' ); - } else { - return $timestamp->format( 'YmdHis' ); - } - } - /** * @return string Comment author */ @@ -198,28 +154,18 @@ class CommentItem extends ThreadItem { } /** - * @return HeadingItem Closest ancestor which is a HeadingItem + * @inheritDoc CommentItemTrait::getHeading + * @suppress PhanTypeMismatchReturnSuperType */ - public function getHeading(): HeadingItem { - $parent = $this; - while ( $parent instanceof CommentItem ) { - $parent = $parent->getParent(); - } - if ( !( $parent instanceof HeadingItem ) ) { - throw new MWException( 'heading parent not found' ); - } - return $parent; + public function getHeading(): ContentHeadingItem { + return $this->traitGetHeading(); } /** - * @return HeadingItem|null Closest heading that can be used for topic subscriptions + * @inheritDoc CommentItemTrait::getSubscribableHeading */ - public function getSubscribableHeading(): ?HeadingItem { - $heading = $this->getHeading(); - while ( $heading instanceof HeadingItem && !$heading->isSubscribable() ) { - $heading = $heading->getParent(); - } - return $heading instanceof HeadingItem ? $heading : null; + public function getSubscribableHeading(): ?ContentHeadingItem { + return $this->traitGetSubscribableHeading(); } /** diff --git a/includes/HeadingItem.php b/includes/ThreadItem/ContentHeadingItem.php similarity index 68% rename from includes/HeadingItem.php rename to includes/ThreadItem/ContentHeadingItem.php index cac178e03..934b28d9f 100644 --- a/includes/HeadingItem.php +++ b/includes/ThreadItem/ContentHeadingItem.php @@ -1,12 +1,17 @@ headingLevel = $this->placeholderHeading ? static::PLACEHOLDER_HEADING_LEVEL : $headingLevel; } - /** - * @inheritDoc - */ - public function jsonSerialize( bool $deep = false, ?callable $callback = null ): array { - return array_merge( parent::jsonSerialize( $deep, $callback ), [ - 'headingLevel' => $this->headingLevel === static::PLACEHOLDER_HEADING_LEVEL ? null : $this->headingLevel, - // Used for topic subscriptions. Not added to CommentItem's yet as there is - // no use case for it. - 'name' => $this->name, - ] ); - } - /** * Get a title based on the hash ID, such that it can be linked to * @@ -88,24 +81,6 @@ class HeadingItem extends ThreadItem { $this->placeholderHeading = $placeholderHeading; } - /** - * Check whether this heading can be used for topic subscriptions. - * - * @return bool - */ - public function isSubscribable(): bool { - return ( - // Placeholder headings have nothing to attach the button to. - !$this->isPlaceholderHeading() && - // We only allow subscribing to level 2 headings, because the user interface for sub-headings - // would be difficult to present. - $this->getHeadingLevel() === 2 && - // Check if the name corresponds to a section that contain no comments (only sub-sections). - // They can't be distinguished from each other, so disallow subscribing. - $this->getName() !== 'h-' - ); - } - /** * @inheritDoc */ diff --git a/includes/ThreadItem.php b/includes/ThreadItem/ContentThreadItem.php similarity index 90% rename from includes/ThreadItem.php rename to includes/ThreadItem/ContentThreadItem.php index 72cc60efb..cca5e3c68 100644 --- a/includes/ThreadItem.php +++ b/includes/ThreadItem/ContentThreadItem.php @@ -1,9 +1,12 @@ range = $range; } - /** - * @param bool $deep Whether to include full serialized comments in the replies key - * @param callable|null $callback Function to call on the returned serialized array, which - * will be passed into the serialized replies as well if $deep is used - * @return array JSON-serializable array - */ - public function jsonSerialize( bool $deep = false, ?callable $callback = null ): array { - // The output of this method can end up in the HTTP cache (Varnish). Avoid changing it; - // and when doing so, ensure that frontend code can handle both the old and new outputs. - // See ThreadItem.static.newFromJSON in JS. - - $array = [ - 'type' => $this->type, - 'level' => $this->level, - 'id' => $this->id, - 'replies' => array_map( static function ( ThreadItem $comment ) use ( $deep, $callback ) { - return $deep ? $comment->jsonSerialize( $deep, $callback ) : $comment->getId(); - }, $this->replies ) - ]; - if ( $callback ) { - $callback( $array, $this ); - } - return $array; - } - /** * Get summary metadata for a thread. * @@ -135,7 +115,7 @@ abstract class ThreadItem implements JsonSerializable { /** * Get the list of thread items in the comment tree below this thread item. * - * @return ThreadItem[] Thread items + * @return ContentThreadItem[] Thread items */ public function getThreadItemsBelow(): array { $threadItems = []; @@ -370,7 +350,7 @@ abstract class ThreadItem implements JsonSerializable { } /** - * @return ThreadItem|null Parent thread item + * @return ContentThreadItem|null Parent thread item */ public function getParent(): ?ThreadItem { return $this->parent; @@ -412,7 +392,7 @@ abstract class ThreadItem implements JsonSerializable { } /** - * @return ThreadItem[] Replies to this thread item + * @return ContentThreadItem[] Replies to this thread item */ public function getReplies(): array { return $this->replies; @@ -433,9 +413,9 @@ abstract class ThreadItem implements JsonSerializable { } /** - * @param ThreadItem $parent + * @param ContentThreadItem $parent */ - public function setParent( ThreadItem $parent ): void { + public function setParent( ContentThreadItem $parent ): void { $this->parent = $parent; } @@ -489,9 +469,9 @@ abstract class ThreadItem implements JsonSerializable { } /** - * @param ThreadItem $reply Reply comment + * @param ContentThreadItem $reply Reply comment */ - public function addReply( ThreadItem $reply ): void { + public function addReply( ContentThreadItem $reply ): void { $this->replies[] = $reply; } } diff --git a/includes/ThreadItem/DatabaseCommentItem.php b/includes/ThreadItem/DatabaseCommentItem.php new file mode 100644 index 000000000..914d5d222 --- /dev/null +++ b/includes/ThreadItem/DatabaseCommentItem.php @@ -0,0 +1,64 @@ +timestamp = $timestamp; + $this->author = $author; + } + + /** + * @inheritDoc + */ + public function getAuthor(): string { + return $this->author; + } + + /** + * @inheritDoc + */ + public function getTimestamp(): DateTimeImmutable { + return new DateTimeImmutable( $this->timestamp ); + } + + /** + * @inheritDoc CommentItemTrait::getHeading + * @suppress PhanTypeMismatchReturnSuperType + */ + public function getHeading(): DatabaseHeadingItem { + return $this->traitGetHeading(); + } + + /** + * @inheritDoc CommentItemTrait::getSubscribableHeading + */ + public function getSubscribableHeading(): ?DatabaseHeadingItem { + return $this->traitGetSubscribableHeading(); + } +} diff --git a/includes/ThreadItem/DatabaseHeadingItem.php b/includes/ThreadItem/DatabaseHeadingItem.php new file mode 100644 index 000000000..cdc067af2 --- /dev/null +++ b/includes/ThreadItem/DatabaseHeadingItem.php @@ -0,0 +1,46 @@ +placeholderHeading = $headingLevel === null; + $this->headingLevel = $this->placeholderHeading ? static::PLACEHOLDER_HEADING_LEVEL : $headingLevel; + } + + /** + * @inheritDoc + */ + public function getHeadingLevel(): int { + return $this->headingLevel; + } + + /** + * @inheritDoc + */ + public function isPlaceholderHeading(): bool { + return $this->placeholderHeading; + } +} diff --git a/includes/ThreadItem/DatabaseThreadItem.php b/includes/ThreadItem/DatabaseThreadItem.php new file mode 100644 index 000000000..2c7648820 --- /dev/null +++ b/includes/ThreadItem/DatabaseThreadItem.php @@ -0,0 +1,101 @@ +name = $name; + $this->id = $id; + $this->type = $type; + $this->parent = $parent; + $this->transcludedFrom = $transcludedFrom; + $this->level = $level; + } + + /** + * @inheritDoc + */ + public function getName(): string { + return $this->name; + } + + /** + * @param DatabaseThreadItem $reply Reply comment + */ + public function addReply( DatabaseThreadItem $reply ): void { + $this->replies[] = $reply; + } + + /** + * @inheritDoc + */ + public function getId(): string { + return $this->id; + } + + /** + * @inheritDoc + */ + public function getType(): string { + return $this->type; + } + + /** + * @inheritDoc + * @return DatabaseThreadItem|null + */ + public function getParent(): ?ThreadItem { + return $this->parent; + } + + /** + * @inheritDoc + * @return DatabaseThreadItem[] + */ + public function getReplies(): array { + return $this->replies; + } + + /** + * @inheritDoc + */ + public function getTranscludedFrom() { + return $this->transcludedFrom; + } + + /** + * @inheritDoc + */ + public function getLevel(): int { + return $this->level; + } +} diff --git a/includes/ThreadItem/HeadingItem.php b/includes/ThreadItem/HeadingItem.php new file mode 100644 index 000000000..c6b9a1136 --- /dev/null +++ b/includes/ThreadItem/HeadingItem.php @@ -0,0 +1,15 @@ + $this->isPlaceholderHeading() ? null : $this->getHeadingLevel(), + // Used for topic subscriptions. Not added to CommentItem's yet as there is + // no use case for it. + 'name' => $this->getName(), + ] ); + } + + /** + * Check whether this heading can be used for topic subscriptions. + * + * @return bool + */ + public function isSubscribable(): bool { + return ( + // Placeholder headings have nothing to attach the button to. + !$this->isPlaceholderHeading() && + // We only allow subscribing to level 2 headings, because the user interface for sub-headings + // would be difficult to present. + $this->getHeadingLevel() === 2 && + // Check if the name corresponds to a section that contain no comments (only sub-sections). + // They can't be distinguished from each other, so disallow subscribing. + $this->getName() !== 'h-' + ); + } +} diff --git a/includes/ThreadItem/ThreadItem.php b/includes/ThreadItem/ThreadItem.php new file mode 100644 index 000000000..9a7d3432e --- /dev/null +++ b/includes/ThreadItem/ThreadItem.php @@ -0,0 +1,53 @@ + $this->getType(), + 'level' => $this->getLevel(), + 'id' => $this->getId(), + 'replies' => array_map( static function ( ThreadItem $comment ) use ( $deep, $callback ) { + return $deep ? $comment->jsonSerialize( $deep, $callback ) : $comment->getId(); + }, $this->getReplies() ) + ]; + if ( $callback ) { + $callback( $array, $this ); + } + return $array; + } +} diff --git a/includes/ThreadItemSet.php b/includes/ThreadItemSet.php index 70ba4acc3..aee29f278 100644 --- a/includes/ThreadItemSet.php +++ b/includes/ThreadItemSet.php @@ -2,58 +2,31 @@ namespace MediaWiki\Extension\DiscussionTools; +use MediaWiki\Extension\DiscussionTools\ThreadItem\CommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\HeadingItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ThreadItem; + /** * Groups thread items (headings and comments) generated by parsing a discussion page. */ -class ThreadItemSet { - - /** @var ThreadItem[] */ - private $threadItems = []; - /** @var CommentItem[] */ - private $commentItems = []; - /** @var ThreadItem[][] */ - private $threadItemsByName = []; - /** @var ThreadItem[] */ - private $threadItemsById = []; - /** @var HeadingItem[] */ - private $threads = []; - +interface ThreadItemSet { /** - * @internal Only used by CommentParser * @param ThreadItem $item + * @internal Only used by CommentParser */ - public function addThreadItem( ThreadItem $item ) { - $this->threadItems[] = $item; - if ( $item instanceof CommentItem ) { - $this->commentItems[] = $item; - } - if ( $item instanceof HeadingItem ) { - $this->threads[] = $item; - } - } + public function addThreadItem( ThreadItem $item ); /** - * @internal Only used by CommentParser * @return bool + * @internal Only used by CommentParser */ - public function isEmpty(): bool { - return !$this->threadItems; - } + public function isEmpty(): bool; /** - * @internal Only used by CommentParser * @param ThreadItem $item + * @internal Only used by CommentParser */ - public function updateIdAndNameMaps( ThreadItem $item ) { - $this->threadItemsByName[ $item->getName() ][] = $item; - - $this->threadItemsById[ $item->getId() ] = $item; - - $legacyId = $item->getLegacyId(); - if ( $legacyId ) { - $this->threadItemsById[ $legacyId ] = $item; - } - } + public function updateIdAndNameMaps( ThreadItem $item ); /** * Get all discussion comments (and headings) within a DOM subtree. @@ -90,18 +63,14 @@ class ThreadItemSet { * * @return ThreadItem[] Thread items */ - public function getThreadItems(): array { - return $this->threadItems; - } + public function getThreadItems(): array; /** * Same as getFlatThreadItems, but only returns the CommentItems * * @return CommentItem[] Comment items */ - public function getCommentItems(): array { - return $this->commentItems; - } + public function getCommentItems(): array; /** * Find ThreadItems by their name @@ -112,9 +81,7 @@ class ThreadItemSet { * @param string $name Name * @return ThreadItem[] Thread items, empty array if not found */ - public function findCommentsByName( string $name ): array { - return $this->threadItemsByName[$name] ?? []; - } + public function findCommentsByName( string $name ): array; /** * Find a ThreadItem by its ID @@ -122,9 +89,7 @@ class ThreadItemSet { * @param string $id ID * @return ThreadItem|null Thread item, null if not found */ - public function findCommentById( string $id ): ?ThreadItem { - return $this->threadItemsById[$id] ?? null; - } + public function findCommentById( string $id ): ?ThreadItem; /** * Group discussion comments into threads and associate replies to original messages. @@ -169,7 +134,5 @@ class ThreadItemSet { * * @return HeadingItem[] Tree structure of comments, top-level items are the headings. */ - public function getThreads(): array { - return $this->threads; - } + public function getThreads(): array; } diff --git a/tests/phpunit/CommentParserTest.php b/tests/phpunit/CommentParserTest.php index 61d324a60..7b3ad8261 100644 --- a/tests/phpunit/CommentParserTest.php +++ b/tests/phpunit/CommentParserTest.php @@ -4,11 +4,11 @@ namespace MediaWiki\Extension\DiscussionTools\Tests; use DateTimeImmutable; use Error; -use MediaWiki\Extension\DiscussionTools\CommentItem; use MediaWiki\Extension\DiscussionTools\CommentUtils; -use MediaWiki\Extension\DiscussionTools\HeadingItem; use MediaWiki\Extension\DiscussionTools\ImmutableRange; -use MediaWiki\Extension\DiscussionTools\ThreadItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentHeadingItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem; use MediaWiki\MediaWikiServices; use stdClass; use Wikimedia\Parsoid\DOM\Element; @@ -54,16 +54,16 @@ class CommentParserTest extends IntegrationTestCase { return implode( '/', $path ); } - private static function serializeComments( ThreadItem $threadItem, Element $root ): stdClass { + private static function serializeComments( ContentThreadItem $threadItem, Element $root ): stdClass { $serialized = new stdClass(); - if ( $threadItem instanceof HeadingItem ) { + if ( $threadItem instanceof ContentHeadingItem ) { $serialized->placeholderHeading = $threadItem->isPlaceholderHeading(); } $serialized->type = $threadItem->getType(); - if ( $threadItem instanceof CommentItem ) { + if ( $threadItem instanceof ContentCommentItem ) { $serialized->timestamp = $threadItem->getTimestampString(); $serialized->author = $threadItem->getAuthor(); } @@ -76,7 +76,7 @@ class CommentParserTest extends IntegrationTestCase { static::getOffsetPath( $root, $range->endContainer, $range->endOffset ) ]; - if ( $threadItem instanceof CommentItem ) { + if ( $threadItem instanceof ContentCommentItem ) { $serialized->signatureRanges = array_map( function ( ImmutableRange $range ) use ( $root ) { return [ static::getOffsetPath( $root, $range->startContainer, $range->startOffset ), @@ -85,7 +85,7 @@ class CommentParserTest extends IntegrationTestCase { }, $threadItem->getSignatureRanges() ); } - if ( $threadItem instanceof HeadingItem ) { + if ( $threadItem instanceof ContentHeadingItem ) { $serialized->headingLevel = $threadItem->getHeadingLevel(); } $serialized->level = $threadItem->getLevel(); diff --git a/tests/phpunit/ThreadItemTest.php b/tests/phpunit/ContentThreadItemTest.php similarity index 85% rename from tests/phpunit/ThreadItemTest.php rename to tests/phpunit/ContentThreadItemTest.php index da64db2cc..de36caf32 100644 --- a/tests/phpunit/ThreadItemTest.php +++ b/tests/phpunit/ContentThreadItemTest.php @@ -3,19 +3,20 @@ namespace MediaWiki\Extension\DiscussionTools\Tests; use DateTimeImmutable; -use MediaWiki\Extension\DiscussionTools\CommentItem; use MediaWiki\Extension\DiscussionTools\CommentUtils; -use MediaWiki\Extension\DiscussionTools\HeadingItem; use MediaWiki\Extension\DiscussionTools\ImmutableRange; -use MediaWiki\Extension\DiscussionTools\ThreadItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentHeadingItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem; +use MediaWiki\Extension\DiscussionTools\ThreadItem\ThreadItem; use MediaWiki\MediaWikiServices; /** - * @coversDefaultClass \MediaWiki\Extension\DiscussionTools\ThreadItem + * @coversDefaultClass \MediaWiki\Extension\DiscussionTools\ThreadItem\ContentThreadItem * * @group DiscussionTools */ -class ThreadItemTest extends IntegrationTestCase { +class ContentThreadItemTest extends IntegrationTestCase { /** * @dataProvider provideAuthors * @covers ::getAuthorsBelow @@ -28,11 +29,11 @@ class ThreadItemTest extends IntegrationTestCase { $node = $doc->createElement( 'div' ); $range = new ImmutableRange( $node, 0, $node, 0 ); - $makeThreadItem = static function ( array $arr ) use ( &$makeThreadItem, $range ): ThreadItem { + $makeThreadItem = static function ( array $arr ) use ( &$makeThreadItem, $range ): ContentThreadItem { if ( $arr['type'] === 'comment' ) { - $item = new CommentItem( 1, $range, [], new DateTimeImmutable(), $arr['author'] ); + $item = new ContentCommentItem( 1, $range, [], new DateTimeImmutable(), $arr['author'] ); } else { - $item = new HeadingItem( $range, 2 ); + $item = new ContentHeadingItem( $range, 2 ); } $item->setId( $arr['id'] ); foreach ( $arr['replies'] as $reply ) { @@ -102,7 +103,7 @@ class ThreadItemTest extends IntegrationTestCase { /** * @dataProvider provideGetText * @covers ::getText - * @covers \MediaWiki\Extension\DiscussionTools\CommentItem::getBodyText + * @covers \MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem::getBodyText * @covers \MediaWiki\Extension\DiscussionTools\ImmutableRange::cloneContents */ public function testGetText( @@ -125,7 +126,7 @@ class ThreadItemTest extends IntegrationTestCase { $output = []; foreach ( $items as $item ) { $output[ $item->getId() ] = CommentUtils::htmlTrim( - $item instanceof CommentItem ? $item->getBodyText( true ) : $item->getText() + $item instanceof ContentCommentItem ? $item->getBodyText( true ) : $item->getText() ); } @@ -148,7 +149,7 @@ class ThreadItemTest extends IntegrationTestCase { /** * @dataProvider provideGetHTML * @covers ::getHTML - * @covers \MediaWiki\Extension\DiscussionTools\CommentItem::getBodyHTML + * @covers \MediaWiki\Extension\DiscussionTools\ThreadItem\ContentCommentItem::getBodyHTML * @covers \MediaWiki\Extension\DiscussionTools\ImmutableRange::cloneContents */ public function testGetHTML( @@ -171,7 +172,7 @@ class ThreadItemTest extends IntegrationTestCase { $output = []; foreach ( $items as $item ) { $output[ $item->getId() ] = CommentUtils::htmlTrim( - $item instanceof CommentItem ? $item->getBodyHTML( true ) : $item->getHTML() + $item instanceof ContentCommentItem ? $item->getBodyHTML( true ) : $item->getHTML() ); } diff --git a/tests/phpunit/MockEventDispatcher.php b/tests/phpunit/MockEventDispatcher.php index 04dee0854..7ececd465 100644 --- a/tests/phpunit/MockEventDispatcher.php +++ b/tests/phpunit/MockEventDispatcher.php @@ -2,8 +2,8 @@ namespace MediaWiki\Extension\DiscussionTools\Tests; +use MediaWiki\Extension\DiscussionTools\ContentThreadItemSet; use MediaWiki\Extension\DiscussionTools\Notifications\EventDispatcher; -use MediaWiki\Extension\DiscussionTools\ThreadItemSet; use MediaWiki\Page\PageIdentity; use MediaWiki\Revision\RevisionRecord; use MediaWiki\User\UserIdentity; @@ -19,16 +19,16 @@ class MockEventDispatcher extends EventDispatcher { * ... expected to be a reference, value given"). * * @param array &$events - * @param ThreadItemSet $oldItemSet - * @param ThreadItemSet $newItemSet + * @param ContentThreadItemSet $oldItemSet + * @param ContentThreadItemSet $newItemSet * @param RevisionRecord $newRevRecord * @param PageIdentity $title * @param UserIdentity $user */ public static function generateEventsFromItemSets( array &$events, - ThreadItemSet $oldItemSet, - ThreadItemSet $newItemSet, + ContentThreadItemSet $oldItemSet, + ContentThreadItemSet $newItemSet, RevisionRecord $newRevRecord, PageIdentity $title, UserIdentity $user