rdbms: Throw when makeWhereFrom2d() receives empty data

This semi-internal method has only four callers, each of which
explicitly or implicitly ensures that $data is empty. For example:

* ApiQueryBacklinksprop: The caller returns early return if $titles
  is empty, and then proceeds to build $map from $titles PageSet.
  I've made it more explicit here for added confidence and to satisfy
  static analysis.
* cleanupPreferences.php: The caller explicilty checks $data
  (as $deleteWhere) before calling makeWhereFrom2d().
* ResourceLoader\WikiModule, the caller checks $batch->isEmpty()
  before calling $batch->constructSet().

More importantly, is that not one of these checks whether
makeWhereFrom2d() returns false, and blindly passes it on to
methods like IDatabase::select() or IDatabase::delete(), which does
not permit boolean false as $where/$conds, but also doesn't check
for it, which seems seems likely to wreak havoc at some point through
casting to empty string or empty array at some point.

Given there are very few callers, and given that the current
responsiblity to check the return value is never excercised, throw
an exception instead to make this code appear less fragile.

Follow-up CR from
<https://gerrit.wikimedia.org/r/c/mediawiki/core/+/892988>

Change-Id: Ifdf3ea54271ea856f2f555a5e087b8f521e348b3

includes/api/ApiQueryBacklinksprop.php

@@ -242,7 +242,7 @@ class ApiQueryBacklinksprop extends ApiQueryGeneratorBase {
 
 		$this->addFieldsIf( 'page_namespace', $miser_ns !== null );
 
-		if ( $hasNS ) {
+		if ( $hasNS && $map ) {
 			// Can't use LinkBatch because it throws away Special titles.
 			// And we already have the needed data structure anyway.
 			$this->addWhere( $db->makeWhereFrom2d( $map, $bl_namespace, $bl_title ) );

includes/libs/rdbms/platform/ISQLPlatform.php

@@ -188,20 +188,34 @@ interface ISQLPlatform {
 	public function makeList( array $a, $mode = self::LIST_COMMA );
 
 	/**
-	 * Build a partial where clause from a 2-d array such as used for LinkBatch.
+	 * Build a "OR" condition with pairs from a two-dimenstional array.
 	 *
-	 * The keys on each level may be either integers or strings, however it's
-	 * assumed that $baseKey is probably an integer-typed column (i.e. integer
-	 * keys are unquoted in the SQL) and $subKey is string-typed (i.e. integer
-	 * keys are quoted as strings in the SQL).
+	 * The associative array should have integer keys relating to the $baseKey field.
+	 * The nested array should have string keys for the $subKey field. The inner
+	 * values are ignored, and are typically boolean true.
 	 *
-	 * @todo Does this actually belong in the library? It seems overly MW-specific.
+	 * Example usage:
+	 * @code
+	 *     $data = [
+	 *         2 => [
+	 *             'Foo' => true,
+	 *             'Bar' => true,
+	 *         ],
+	 *         3 => [
+	 *             'Quux' => true,
+	 *         ],
+	 *     ];
+	 *     // (page_namespace = 2 AND page_title IN ('Foo','Bar'))
+	 *     // OR (page_namespace = 3 AND page_title = 'Quux')
 	 *
-	 * @param array $data Organized as 2-d
-	 *    [ baseKeyVal => [ subKeyVal => [ignored], ... ], ... ]
+	 * @todo This is effectively specific to MediaWiki's LinkBatch.
+	 * Consider deprecating or generalising slightly.
+	 *
+	 * @param array $data Nested array, must be non-empty
+	 * @phan-param non-empty-array $data
 	 * @param string $baseKey Field name to match the base-level keys to (eg 'pl_namespace')
 	 * @param string $subKey Field name to match the sub-level keys to (eg 'pl_title')
-	 * @return string|false SQL fragment, or false if no items in array
+	 * @return string SQL fragment
 	 */
 	public function makeWhereFrom2d( $data, $baseKey, $subKey );
 

includes/libs/rdbms/platform/SQLPlatform.php

@@ -285,7 +285,6 @@ class SQLPlatform implements ISQLPlatform {
 
 	public function makeWhereFrom2d( $data, $baseKey, $subKey ) {
 		$conds = [];
-
 		foreach ( $data as $base => $sub ) {
 			if ( count( $sub ) ) {
 				$conds[] = $this->makeList(
@@ -295,12 +294,11 @@ class SQLPlatform implements ISQLPlatform {
 			}
 		}
 
-		if ( $conds ) {
-			return $this->makeList( $conds, self::LIST_OR );
-		} else {
-			// Nothing to search for...
-			return false;
+		if ( !$conds ) {
+			throw new InvalidArgumentException( "Data for $baseKey and $subKey must be non-empty" );
 		}
+
+		return $this->makeList( $conds, self::LIST_OR );
 	}
 
 	public function factorConds( $condsArray ) {

tests/phpunit/unit/includes/libs/rdbms/platform/SQLPlatformTest.php

@@ -1153,4 +1153,52 @@ class SQLPlatformTest extends PHPUnit\Framework\TestCase {
 		$output = $this->platform->buildIntegerCast( 'fieldName' );
 		$this->assertSame( 'CAST( fieldName AS INTEGER )', $output );
 	}
+
+	public static function provideMakeWhereFrom2d() {
+		yield [
+			[ 2 => [ 'Foo' => true, 'Bar' => true ], 4 => [ 'Quux' => true ] ],
+			"(ns = 2 AND title IN ('Foo','Bar') ) OR (ns = 4 AND title = 'Quux')",
+		];
+		yield [
+			[ 2 => [ 'Foo' => true, 'Bar' => true ], 4 => [] ],
+			"(ns = 2 AND title IN ('Foo','Bar') )",
+		];
+		yield [
+			[ 2 => [ 'Foo' => true ], 4 => [] ],
+			"(ns = 2 AND title = 'Foo')",
+		];
+		yield [
+			[ 2 => [ 'Foo' => true ] ],
+			"(ns = 2 AND title = 'Foo')",
+		];
+		yield [
+			[ 'x' => [ 42 ] ],
+			"(ns = 'x' AND title = '0')",
+		];
+	}
+
+	public static function provideMakeWhereFrom2dInvalid() {
+		yield [
+			[],
+			[ 2 => [], 4 => [] ],
+		];
+	}
+
+	/**
+	 * @dataProvider provideMakeWhereFrom2d
+	 */
+	public function testMakeWhereFrom2d( $data, $expected ) {
+		$this->assertSame(
+			$expected,
+			$this->platform->makeWhereFrom2d( $data, 'ns', 'title' )
+		);
+	}
+
+	/**
+	 * @dataProvider provideMakeWhereFrom2dInvalid
+	 */
+	public function testMakeWhereFrom2dInvalid( $data ) {
+		$this->expectException( InvalidArgumentException::class );
+		$this->platform->makeWhereFrom2d( $data, 'ns', 'title' );
+	}
 }