Pmiazga has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/382609 )
Change subject: [POC] Separation of concerns in BookRenderer
......................................................................
[POC] Separation of concerns in BookRenderer
Changes:
- introduced BookData class to store $pages, $collection
and $metadata all together. Those three params are usually
passed around together so we can use a wrapper
- allow BookData class to handle bulding/retrieving pages,
collection data and metadata
- simplified $collection array handling as we use only 'title',
'subtitle' and 'items' keys. Rendering classes don't have to know
how $collection array is built
- introduce BookMetadata class to store $metadata variable
- only BookMetadata know how to build itself so it minimizes
scenarios when different parts of the code modify $metadata
variable in strange way
- simplified return statements so it doesn't use complex array
functions
Change-Id: I49228dbba768b8190ee2efb833abd87d18fe7318
---
M Collection.php
A includes/BookData.php
A includes/BookMetadata.php
M includes/BookRenderer.php
M includes/BookRenderingMediator.php
M includes/DataProvider.php
6 files changed, 265 insertions(+), 91 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Collection
refs/changes/09/382609/1
diff --git a/Collection.php b/Collection.php
index b3751e2..2c07123 100644
--- a/Collection.php
+++ b/Collection.php
@@ -198,6 +198,8 @@
= __DIR__ . '/includes/BookRenderingMediator.php';
$wgAutoloadClasses[\MediaWiki\Extensions\Collection\MessageBoxHelper::class ]
= __DIR__ . '/includes/MessageBoxHelper.php';
+$wgAutoloadClasses[\MediaWiki\Extensions\Collection\BookData::class ]
+ = __DIR__ . '/includes/BookData.php';
$wgAutoloadClasses['CollectionPageTemplate'] = __DIR__ .
'/templates/CollectionPageTemplate.php';
$wgAutoloadClasses['CollectionListTemplate'] = __DIR__ .
'/templates/CollectionListTemplate.php';
diff --git a/includes/BookData.php b/includes/BookData.php
new file mode 100644
index 0000000..3ade08f
--- /dev/null
+++ b/includes/BookData.php
@@ -0,0 +1,105 @@
+<?php
+
+namespace MediaWiki\Extensions\Collection;
+
+class BookData {
+
+ /**
+ * @var array
+ */
+ private $collection;
+ /**
+ * @var array
+ */
+ private $pages;
+ /**
+ * @var BookMetadata
+ */
+ private $metadata;
+ /**
+ * @var bool
+ */
+ private $hasChapters;
+ /**
+ * @var int
+ */
+ private $articlesCount;
+ /**
+ * @var array
+ */
+ private $outline = [];
+
+ public function __construct( $collection, $pages, BookMetadata
$metadata ) {
+ $this->collection = $collection;
+ $this->pages = $pages;
+ $this->metadata = $metadata;
+ }
+
+ public function getItems() {
+ return $this->collection['items'];
+ }
+
+ public function getTtile() {
+ return $this->collection['title'];
+ }
+
+ public function getSubtitle() {
+ return $this->collection['subtitle'];
+ }
+
+ public function getPages() {
+ return $this->pages;
+ }
+
+ public function getMetadata() {
+ return $this->metadata;
+ }
+
+ public function hasChapters() {
+ if ( $this->hasChapters === null ) {
+ $this->hasChapters = (bool)array_filter(
$this->collection['items'], function ( $item ) {
+ return $item['type'] === 'chapter';
+ } );
+ }
+ return $this->hasChapters;
+ }
+
+ public function getArticlesCount() {
+ if ( $this->articlesCount === null ) {
+ $this->articlesCount = count(
+ array_filter( $this->collection['items'],
function ( $item ) {
+ return $item['type'] === 'article';
+ } ) );
+ }
+ return $this->articlesCount;
+ }
+
+ public function overrideSections( $dbkey, $newSections ) {
+ $this->metadata->setSections( $dbkey, $newSections );
+ }
+
+ public function addOutline( $outline ) {
+ $this->outline[] = $outline;
+ }
+
+ public function getOutline() {
+ return $this->outline;
+ }
+
+ public function getDisplayTitleFor( $dbkey ) {
+ return $this->metadata->getDisplayTitle( $dbkey );
+ }
+
+ public function getSectionsFor( $dbkey ) {
+ return $this->metadata->getSections( $dbkey );
+ }
+
+ public function getContributors() {
+ return $this->metadata->getContributors();
+ }
+
+ public function getPage( $dbkey ) {
+ return $this->pages[$dbkey];
+ }
+
+}
diff --git a/includes/BookMetadata.php b/includes/BookMetadata.php
new file mode 100644
index 0000000..b59c7bb
--- /dev/null
+++ b/includes/BookMetadata.php
@@ -0,0 +1,77 @@
+<?php
+
+namespace MediaWiki\Extensions\Collection;
+
+class BookMetadata {
+
+ private $metadata = [
+ 'displaytitle' => [],
+ 'sections' => [],
+ 'contributors' => [],
+ 'modules' => [],
+ 'modulescripts' => [],
+ 'modulestyles' => [],
+ 'jsconfigvars' => [],
+ ];
+
+ public function addContributor( $name, $id ) {
+ $this->metadata['contributors'][$name] = $id;
+ }
+
+ public function getContributors() {
+ return $this->metadata['contributors'];
+ }
+
+ public function setDisplayTitle( $dbkey, $displayTitle ) {
+ $this->metadata['displaytitle'][$dbkey] = $displayTitle;
+ }
+
+ public function getDisplayTitle( $dbkey ) {
+ return $this->metadata['displaytitle'][$dbkey];
+ }
+
+ public function setSections( $dbkey, $sections ) {
+ $this->metadata['sections'][$dbkey] = $sections;
+ }
+
+ public function getSections( $dbkey ) {
+ return $this->metadata['sections'][$dbkey];
+ }
+
+ public function mergeModules( $modules ) {
+ $this->mergeData( 'modules', $modules );
+ }
+
+ public function mergeModuleScripts( $modules ) {
+ $this->mergeData( 'modulescripts', $modules );
+ }
+
+ public function mergeModuleStyles( $modules ) {
+ $this->mergeData( 'modulestyles', $modules );
+ }
+
+ public function mergeModuleJSConfigVars( $modules ) {
+ $this->mergeData( 'jsconfigvars', $modules );
+ }
+
+ public function getModules() {
+ return $this->metadata['modules'];
+ }
+
+ public function getModuleScripts() {
+ return $this->metadata['modulescripts'];
+ }
+
+ public function getModuleStyles() {
+ return $this->metadata['modulestyles'];
+ }
+
+ public function getJSConfigVars() {
+ return $this->metadata['jsconfigvars'];
+ }
+
+ private function mergeData( $field, $data ) {
+ $this->metadata[$field] = array_merge( $this->metadata[$field],
$data );
+ }
+
+}
diff --git a/includes/BookRenderer.php b/includes/BookRenderer.php
index 1825ca7..d3b4531 100644
--- a/includes/BookRenderer.php
+++ b/includes/BookRenderer.php
@@ -26,21 +26,11 @@
/**
* Generate the concatenated page.
- * @param array[] $collection Collection, as returned by
CollectionSession::getCollection().
- * @param string[] $pages Map of prefixed DB key => Parsoid HTML.
- * @param array[] &$metadata Map of prefixed DB key => metadata, as
returned by fetchMetadata().
- * Section data will be updated to account for heading level and id
changes.
- * Also, an outline will be added (see renderCoverAndToc() for
format).
+ * @param BookData $bookData
* @return string HTML of the rendered book (without body/head).
*/
- public function renderBook( $collection, $pages, &$metadata ) {
- $hasChapters = (bool)array_filter( $collection['items'],
function ( $item ) {
- return $item['type'] === 'chapter';
- } );
- $articleCount = count( array_filter( $collection['items'],
function ( $item ) {
- return $item['type'] === 'article';
- } ) );
-
+ public function renderBook( BookData $bookData ) {
+ $items = $bookData->getItems();
$final = '';
$headingCounter = new HeadingCounter();
@@ -50,9 +40,9 @@
$formatter = new \RemexHtml\Serializer\HtmlFormatter();
$serializer = new \RemexHtml\Serializer\Serializer( $formatter
);
$munger = new RemexCollectionMunger( $serializer, [
- 'topHeadingLevel' => $hasChapters ? 3 : 2,
+ 'topHeadingLevel' => $bookData->hasChapters() ? 3 : 2,
] );
- foreach ( $collection['items'] as $item ) {
+ foreach ( $items as $item ) {
if ( $item['type'] === 'chapter' ) {
$final .= Html::element( 'h1', [
'id' => 'mw-book-chapter-' .
Sanitizer::escapeIdForAttribute( $item['title'] ),
@@ -62,21 +52,22 @@
} elseif ( $item['type'] === 'article' ) {
$title = Title::newFromText( $item['title'] );
$dbkey = $title->getPrefixedDBkey();
- $html = $this->getBodyContents( $pages[$dbkey]
);
+ $html = $this->getBodyContents(
$bookData->getPage( $dbkey ) );
$headingAttribs = [
'id' => 'mw-book-article-' . $dbkey,
'class' => 'mw-book-article',
];
$mungerOptions = [];
- if ( $articleCount > 1 ) {
+ if ( $bookData->getArticlesCount() > 1 ) {
$mungerOptions['sectionNumberPrefix'] =
$headingAttribs['data-mw-sectionnumber']
=
$headingCounter->incrementAndGet( -1 );
}
- $final .= Html::rawElement( 'h2',
$headingAttribs, $metadata['displaytitle'][$dbkey] ) . "\n";
+ $final .= Html::rawElement( 'h2',
$headingAttribs,
+ $bookData->getDisplayTitleFor(
$dbkey ) ). "\n";
+ $sections = $bookData->getSectionsFor( $dbkey );
- $munger->startCollectionSection( './' . $dbkey,
$metadata['sections'][$dbkey],
- $headingCounter );
+ $munger->startCollectionSection( './' . $dbkey,
$sections, $headingCounter );
$treeBuilder = new
\RemexHtml\TreeBuilder\TreeBuilder( $munger, [] );
$dispatcher = new
\RemexHtml\TreeBuilder\Dispatcher( $treeBuilder );
$tokenizer = new
\RemexHtml\Tokenizer\Tokenizer( $dispatcher, $html, [
@@ -93,105 +84,101 @@
$final .= Html::openElement( 'article' )
. substr( $serializer->getResult(), 15
) // strip "<!DOCTYPE html>"
. Html::closeElement( 'article' );
+
+ // munger is modifing $sections array, we have
to update it
+ $bookData->overrideSections( $dbkey, $sections
);
}
}
+ $this->buildOutline( $bookData );
- $final = $this->renderCoverAndToc( $collection, $metadata )
+ $final = $this->renderCoverAndToc( $bookData )
. $final
- . $this->renderContributors( $metadata,
$headingCounter->incrementAndGetTopLevel() );
+ . $this->renderContributors( $bookData,
$headingCounter->incrementAndGetTopLevel() );
return $final;
}
- /**
- * Generate HTML for book cover page and table of contents.
- * @param array $collection Collection, as returned by
CollectionSession::getCollection().
- * @param array[] $metadata Map of prefixed DB key => metadata, as
returned by fetchMetadata().
- * An outline will be added which is similar to sections but flat and
each item has the fields
- * - text: text of the outline item (article title, section title
etc)
- * - type: 'chapter', 'article', 'section' or 'contributors'
- * - level: heading level or -2 for chapter, -1 for article
- * - anchor: id of the document node which the outline item refers
to
- * - number: a hierarchical section number (something like "1.2.3")
- * @return string HTML to prepend to the book.
- */
- private function renderCoverAndToc( $collection, &$metadata ) {
- $hasChapters = (bool)array_filter( $collection['items'],
function ( $item ) {
- return $item['type'] === 'chapter';
- } );
- $articleCount = count( array_filter( $collection['items'],
function ( $item ) {
- return $item['type'] === 'article';
- } ) );
+ private function buildOutline( BookData $bookData ) {
$headingCounter = new HeadingCounter();
- $outline = [];
- foreach ( $collection['items'] as $item ) {
+ $items = $bookData->getItems();
+
+ foreach ( $items as $item ) {
if ( $item['type'] === 'chapter' ) {
- $outline[] = [
+ $bookData->addOutline( [
'text' => htmlspecialchars(
$item['title'], ENT_QUOTES ),
'type' => 'chapter',
'level' => -2,
'anchor' => 'mw-book-chapter-' .
Sanitizer::escapeIdForAttribute( $item['title'] ),
'number' =>
$headingCounter->incrementAndGet( -2 ),
- ];
+ ] );
} elseif ( $item['type'] === 'article' ) {
$title = Title::newFromText( $item['title'] );
$dbkey = $title->getPrefixedDBkey();
- if ( $articleCount > 1 ) {
- $outline[] = [
- 'text' =>
$metadata['displaytitle'][$dbkey],
+ if ( $bookData->getArticlesCount() > 1 ) {
+ $bookData->addOutline( [
+ 'text' =>
$bookData->getDisplayTitleFor( $dbkey ),
'type' => 'article',
'level' => -1,
'anchor' => 'mw-book-article-'
. $dbkey,
'number' =>
$headingCounter->incrementAndGet( -1 ),
- ];
+ ] );
}
- foreach ( $metadata['sections'][$dbkey] as
$section ) {
- $outline[] = [
+ $sections = $bookData->getSectionsFor( $dbkey );
+ foreach ( $sections as $section ) {
+ $bookData->addOutline( [
'text' => $section['title'],
'type' => 'section',
'level' => $section['level'],
'anchor' => $section['id'],
'number' =>
$headingCounter->incrementAndGet( $section['level'] ),
- ];
+ ] );
}
} else {
throw new LogicException( 'Unknown collection
item type: ' . $item['type'] );
}
}
- if ( $hasChapters ) {
+ if ( $bookData->hasChapters() ) {
$contributorsLevel = -2;
- } elseif ( $articleCount > 1 ) {
+ } elseif ( $bookData->getArticlesCount() > 1 ) {
$contributorsLevel = -1;
} else {
$contributorsLevel = 0;
}
- $outline[] = [
+ $bookData->addOutline( [
'text' => wfMessage( 'coll-contributors-title'
)->text(),
'type' => 'contributors',
'level' => $contributorsLevel,
'anchor' => 'mw-book-contributors',
'number' => $headingCounter->incrementAndGetTopLevel(),
- ];
- $metadata['outline'] = $outline;
+ ] );
+ }
+ /**
+ * Generate HTML for book cover page and table of contents.
+ * @param BookData $bookData
+ * @return string HTML to prepend to the book.
+ */
+ private function renderCoverAndToc( BookData $bookData ) {
return $this->templateParser->processTemplate( 'toc',
$this->fixTemplateData( [
- 'title' => $collection['title'],
- 'subtitle' => $collection['subtitle'],
+ 'title' => $bookData->getTtile(),
+ 'subtitle' => $bookData->getSubtitle(),
'toctitle' => wfMessage( 'coll-toc-title' )->text(),
- 'tocitems' => $this->getNestedOutline( $outline ),
+ 'tocitems' => $this->getNestedOutline(
$bookData->getOutline() ),
] ) );
}
/**
* Generate HTML for the list of contributors.
- * @param array[] $metadata Map of prefixed DB key => metadata, as
returned by fetchMetadata().
+ * @param BookData $bookData
* @param string $sectionNumber The section number for the contributors
section, if any.
* @return string HTML to append to the book.
*/
- private function renderContributors( $metadata, $sectionNumber = null )
{
+ private function renderContributors( BookData $bookData, $sectionNumber
= null ) {
+ $contributors = $bookData->getContributors();
+
$list = array_map( function ( $name ) {
return Html::element( 'li', [], $name );
- }, array_keys( $metadata['contributors'] ) );
+ }, array_keys( $contributors ) );
$attribs = [ 'id' => 'mw-book-contributors' ];
if ( $sectionNumber ) {
diff --git a/includes/BookRenderingMediator.php
b/includes/BookRenderingMediator.php
index 9d30550..6b4325c 100644
--- a/includes/BookRenderingMediator.php
+++ b/includes/BookRenderingMediator.php
@@ -58,25 +58,35 @@
public function getBook( array $collection ) {
$dataProvider = new DataProvider( $this->restServiceClient );
$dataProvider->setLogger( $this->logger );
- $bookRenderer = new BookRenderer( $this->templateParser );
$status = $dataProvider->fetchPages( $collection );
- if ( $status->isOK() ) {
- $pages = $status->getValue();
- } else {
+ if ( !$status->isOK() ) {
$message = Status::wrap( $status )->getMessage( false,
'coll-rendererror-fetch-wrapper' );
throw new ErrorPageError( 'coll-rendererror-title',
$message );
}
+
+ $pages = $status->getValue();
$status = $dataProvider->fetchMetadata( array_keys( $pages ) );
- if ( $status->isOK() ) {
- $metadata = $status->getValue();
- } else {
+ if ( !$status->isOK() ) {
throw new ErrorPageError( 'coll-rendererror-title',
Status::wrap( $status )->getMessage() );
}
- $html = $bookRenderer->renderBook( $collection, $pages,
$metadata );
+ /** @var BookMetadata $bookMetadata */
+ $bookMetadata = $status->getValue();
+ $bookData = new BookData( $collection, $pages, $bookMetadata );
+ return $this->getBookContent( $bookData );
+ }
- $fields = [ 'html', 'modules', 'modulescripts', 'modulestyles',
'jsconfigvars' ];
- return array_intersect_key( [ 'html' => $html ] + $metadata,
array_fill_keys( $fields, null ) );
+ private function getBookContent( BookData $bookData ) {
+ $bookRenderer = new BookRenderer( $this->templateParser );
+ $html = $bookRenderer->renderBook( $bookData );
+ $metadata = $bookData->getMetadata();
+ return [
+ 'html' => $html,
+ 'modules' => $metadata->getModules(),
+ 'modulescripts' => $metadata->getModuleScripts(),
+ 'modulestyles' => $metadata->getModuleStyles(),
+ 'jsconfigvars' => $metadata->getJSConfigVars()
+ ];
}
/**
diff --git a/includes/DataProvider.php b/includes/DataProvider.php
index dacef53..ee7df61 100644
--- a/includes/DataProvider.php
+++ b/includes/DataProvider.php
@@ -123,15 +123,7 @@
* - jsconfigvars: [ var, ... ]
*/
public function fetchMetadata( $dbkeys ) {
- $metadata = [
- 'displaytitle' => [],
- 'sections' => [],
- 'contributors' => [],
- 'modules' => [],
- 'modulescripts' => [],
- 'modulestyles' => [],
- 'jsconfigvars' => [],
- ];
+ $metadata = new BookMetadata();
// get contributors
$params = [
@@ -148,7 +140,7 @@
$params = $continue + $params;
foreach ( $data['query']['pages'] as $page ) {
foreach ( $page['contributors'] as $key =>
$contrib ) {
-
$metadata['contributors'][$contrib['name']] = $contrib['userid'];
+ $metadata->addContributor(
$contrib['name'], $contrib['userid'] );
}
}
} while ( $continue );
@@ -161,18 +153,19 @@
'prop' =>
'sections|displaytitle|modules|jsconfigvars',
'page' => $dbkey,
] );
- $metadata['displaytitle'][$dbkey] =
$data['parse']['displaytitle'];
- $metadata['sections'][$dbkey] = array_map( function (
$sectionData ) {
+ $metadata->setDisplayTitle( $dbkey,
$data['parse']['displaytitle'] );
+ $metadata->setSections( $dbkey, array_map( function (
$sectionData ) {
return [
'title' => $sectionData['line'],
'id' => $sectionData['anchor'],
- 'level' => intval(
$sectionData['level'] ),
+ 'level' => (int)$sectionData['level'],
];
- }, $data['parse']['sections'] );
- foreach ( [ 'modules', 'modulescripts', 'modulestyles',
'jsconfigvars' ] as $field ) {
- // let's hope there is no conflict in
jsconfigvars...
- $metadata[$field] = array_merge(
$metadata[$field], $data['parse'][$field] );
- }
+ }, $data['parse']['sections'] ) );
+
+ $metadata->mergeModules( $data['parse']['modules'] );
+ $metadata->mergeModuleScripts(
$data['parse']['modules'] );
+ $metadata->mergeModuleStyles( $data['parse']['modules']
);
+ $metadata->mergeModuleJSConfigVars(
$data['parse']['modules'] );
}
return StatusValue::newGood( $metadata );
--
To view, visit https://gerrit.wikimedia.org/r/382609
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I49228dbba768b8190ee2efb833abd87d18fe7318
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Collection
Gerrit-Branch: master
Gerrit-Owner: Pmiazga <pmia...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
