From 7f687cc858ed664a0e614f93b461d874f1cd1ef2 Mon Sep 17 00:00:00 2001
From: Christian Aistleitner <christian@quelltextlich.at>
Date: Fri, 20 Feb 2015 00:13:07 +0100
Subject: [PATCH] Allow to set stub read buffer size for TextPassDumper

The default buffer size of 512KB seems to unconveniently high for
checkpoint tests. To be able to speed up tests in a follow-up commit,
we allow callers to set the buffer size to use for reads of the stub.

Bug: T70653
Change-Id: Ib63e89fac2abaac8feb542839d4d8e53c917ebe1
---
 maintenance/backupTextPass.inc                |  9 ++-
 maintenance/dumpTextPass.php                  |  2 +
 .../maintenance/backupTextPassTest.php        | 62 ++++++++++++++++++-
 3 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/maintenance/backupTextPass.inc b/maintenance/backupTextPass.inc
index 85ebd51bfe..d83f1fccc1 100644
--- a/maintenance/backupTextPass.inc
+++ b/maintenance/backupTextPass.inc
@@ -48,6 +48,8 @@ class TextPassDumper extends BackupDumper {
 	protected $maxConsecutiveFailedTextRetrievals = 200;
 	protected $failureTimeout = 5; // Seconds to sleep after db failure
 
+	protected $bufferSize = 524288; // In bytes. Maximum size to read from the stub in on go.
+
 	protected $php = "php";
 	protected $spawn = false;
 
@@ -186,6 +188,10 @@ class TextPassDumper extends BackupDumper {
 		$url = $this->processFileOpt( $val, $param );
 
 		switch ( $opt ) {
+			case 'buffersize':
+				// Lower bound for xml reading buffer size is 4 KB
+				$this->bufferSize = max( intval( $val ), 4 * 1024 );
+				break;
 			case 'prefetch':
 				require_once "$IP/maintenance/backupPrefetch.inc";
 				$this->prefetch = new BaseDump( $url );
@@ -368,12 +374,11 @@ class TextPassDumper extends BackupDumper {
 		xml_set_character_data_handler( $parser, array( &$this, 'characterData' ) );
 
 		$offset = 0; // for context extraction on error reporting
-		$bufferSize = 512 * 1024;
 		do {
 			if ( $this->checkIfTimeExceeded() ) {
 				$this->setTimeExceeded();
 			}
-			$chunk = fread( $input, $bufferSize );
+			$chunk = fread( $input, $this->bufferSize );
 			if ( !xml_parse( $parser, $chunk, feof( $input ) ) ) {
 				wfDebug( "TextDumpPass::readDump encountered XML parsing error\n" );
 
diff --git a/maintenance/dumpTextPass.php b/maintenance/dumpTextPass.php
index 7c176071ad..bde5a07623 100644
--- a/maintenance/dumpTextPass.php
+++ b/maintenance/dumpTextPass.php
@@ -59,6 +59,8 @@ Options:
   --server=h  Force reading from MySQL server h
   --current	  Base ETA on number of pages in database instead of all revisions
   --spawn	  Spawn a subprocess for loading text records
+  --buffersize=<size> Buffer size in bytes to use for reading the stub.
+              (Default: 512KB, Minimum: 4KB)
   --help      Display this help message
 ENDS
 	);
diff --git a/tests/phpunit/maintenance/backupTextPassTest.php b/tests/phpunit/maintenance/backupTextPassTest.php
index 0a977dc29a..8ed8e1068f 100644
--- a/tests/phpunit/maintenance/backupTextPassTest.php
+++ b/tests/phpunit/maintenance/backupTextPassTest.php
@@ -3,13 +3,13 @@
 require_once __DIR__ . "/../../../maintenance/backupTextPass.inc";
 
 /**
- * Tests for page dumps of BackupDumper
+ * Tests for TextPassDumper that rely on the database
  *
  * @group Database
  * @group Dump
  * @covers TextPassDumper
  */
-class TextPassDumperTest extends DumpTestCase {
+class TextPassDumperDatabaseTest extends DumpTestCase {
 
 	// We'll add several pages, revision and texts. The following variables hold the
 	// corresponding ids.
@@ -615,3 +615,61 @@ class BackupTextPassTestModelHandler extends TextContentHandler {
 	}
 
 }
+
+/**
+ * Tests for TextPassDumper that do not rely on the database
+ *
+ * (As the Database group is only detected at class level (not method level), we
+ * cannot bring this test case's tests into the above main test case.)
+ *
+ * @group Dump
+ * @covers TextPassDumper
+ */
+class TextPassDumperDatabaselessTest extends MediaWikiLangTestCase {
+	/**
+	 * Ensures that setting the buffer size is effective.
+	 *
+	 * @dataProvider bufferSizeProvider
+	 */
+	function testBufferSizeSetting( $expected, $size, $msg ) {
+		$dumper = new TextPassDumperAccessor( array( "--buffersize=" . $size ) );
+		$this->assertEquals( $expected, $dumper->getBufferSize(), $msg);
+	}
+
+	/**
+	 * Ensures that setting the buffer size is effective.
+	 *
+	 * @dataProvider bufferSizeProvider
+	 */
+	function bufferSizeProvider() {
+		// expected, bufferSize to initialize with, message
+		return array(
+			array( 512 * 1024, 512 * 1024, "Setting 512KB is not effective" ),
+			array( 8192, 8192, "Setting 8KB is not effective" ),
+			array( 4096, 2048, "Could set buffer size below lower bound" )
+		);
+	}
+}
+
+/**
+ * Accessor for internal state of TextPassDumper
+ *
+ * Do not warrentless add getters here.
+ */
+class TextPassDumperAccessor extends TextPassDumper {
+	/**
+	 * Gets the bufferSize.
+	 *
+	 * If bufferSize setting does not work correctly, testCheckpoint... tests
+	 * fail and point in the wrong direction. To aid in troubleshooting when
+	 * testCheckpoint... tests break at some point in the future, we test the
+	 * bufferSize setting, hence need this accessor.
+	 *
+	 * (Yes, bufferSize is internal state of the TextPassDumper, but aiding
+	 * debugging of testCheckpoint... in the future seems to be worth testing
+	 * against it nonetheless.)
+	 */
+	public function getBufferSize() {
+		return $this->bufferSize;
+	}
+}
-- 
2.20.1