Aaron Schulz has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/311311

Change subject: Various small cleanups to ResultWrapper
......................................................................

Various small cleanups to ResultWrapper

* Make FakeResultWrapper call super to avoid IDEA warnings
* Lots of documentation cleanups

Change-Id: Ifcd7163890fa8c7718e88c4244cc38371ed624fc
---
M includes/db/DatabaseSqlite.php
M includes/libs/rdbms/database/resultwrapper/FakeResultWrapper.php
M includes/libs/rdbms/database/resultwrapper/MssqlResultWrapper.php
M includes/libs/rdbms/database/resultwrapper/ResultWrapper.php
4 files changed, 67 insertions(+), 68 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/11/311311/1

diff --git a/includes/db/DatabaseSqlite.php b/includes/db/DatabaseSqlite.php
index 28fb5b5..8c4d4d3 100644
--- a/includes/db/DatabaseSqlite.php
+++ b/includes/db/DatabaseSqlite.php
@@ -296,11 +296,11 @@
                $res = $this->mConn->query( $sql );
                if ( $res === false ) {
                        return false;
-               } else {
-                       $r = $res instanceof ResultWrapper ? $res->result : 
$res;
-                       $this->mAffectedRows = $r->rowCount();
-                       $res = new ResultWrapper( $this, $r->fetchAll() );
                }
+
+               $r = $res instanceof ResultWrapper ? $res->result : $res;
+               $this->mAffectedRows = $r->rowCount();
+               $res = new ResultWrapper( $this, $r->fetchAll() );
 
                return $res;
        }
@@ -1065,4 +1065,4 @@
                return 'SQLite ' . (string)$this->mConn->getAttribute( 
PDO::ATTR_SERVER_VERSION );
        }
 
-} // end DatabaseSqlite class
+}
diff --git a/includes/libs/rdbms/database/resultwrapper/FakeResultWrapper.php 
b/includes/libs/rdbms/database/resultwrapper/FakeResultWrapper.php
index 774def8..1a046cf 100644
--- a/includes/libs/rdbms/database/resultwrapper/FakeResultWrapper.php
+++ b/includes/libs/rdbms/database/resultwrapper/FakeResultWrapper.php
@@ -4,35 +4,19 @@
  * doesn't go anywhere near an actual database.
  */
 class FakeResultWrapper extends ResultWrapper {
-       /** @var array */
-       public $result = [];
-
-       /** @var null And it's going to stay that way :D */
-       protected $db = null;
-
-       /** @var int */
-       protected $pos = 0;
-
-       /** @var array|stdClass|bool */
-       protected $currentRow = null;
+       /** @var $result stdClass[] */
 
        /**
-        * @param array $array
+        * @param stdClass[] $rows
         */
-       function __construct( $array ) {
-               $this->result = $array;
+       function __construct( array $rows ) {
+               parent::__construct( null, $rows );
        }
 
-       /**
-        * @return int
-        */
        function numRows() {
                return count( $this->result );
        }
 
-       /**
-        * @return array|bool
-        */
        function fetchRow() {
                if ( $this->pos < count( $this->result ) ) {
                        $this->currentRow = $this->result[$this->pos];
@@ -54,10 +38,6 @@
        function free() {
        }
 
-       /**
-        * Callers want to be able to access fields with $this->fieldName
-        * @return bool|stdClass
-        */
        function fetchObject() {
                $this->fetchRow();
                if ( $this->currentRow ) {
@@ -72,9 +52,6 @@
                $this->currentRow = null;
        }
 
-       /**
-        * @return bool|stdClass
-        */
        function next() {
                return $this->fetchObject();
        }
diff --git a/includes/libs/rdbms/database/resultwrapper/MssqlResultWrapper.php 
b/includes/libs/rdbms/database/resultwrapper/MssqlResultWrapper.php
index cccb8f1..768511b 100644
--- a/includes/libs/rdbms/database/resultwrapper/MssqlResultWrapper.php
+++ b/includes/libs/rdbms/database/resultwrapper/MssqlResultWrapper.php
@@ -1,5 +1,6 @@
 <?php
 class MssqlResultWrapper extends ResultWrapper {
+       /** @var integer|null */
        private $mSeekTo = null;
 
        /**
diff --git a/includes/libs/rdbms/database/resultwrapper/ResultWrapper.php 
b/includes/libs/rdbms/database/resultwrapper/ResultWrapper.php
index 4843d02..53109c8 100644
--- a/includes/libs/rdbms/database/resultwrapper/ResultWrapper.php
+++ b/includes/libs/rdbms/database/resultwrapper/ResultWrapper.php
@@ -1,30 +1,43 @@
 <?php
 /**
- * Result wrapper for grabbing data queried by someone else
+ * Result wrapper for grabbing data queried from an IDatabase object
+ *
+ * Note that using the Iterator methods in combination with the non-Iterator
+ * DB result iteration functions may cause rows to be skipped or repeated.
+ *
+ * By default, this will use the iteration methods of the IDatabase handle if 
provided.
+ * Subclasses can override methods to make it solely work on the result 
resource instead.
+ * If no database is provided, and the subclass does not override the DB 
iteration methods,
+ * then a RuntimeException will be thrown when iteration is attempted.
+ *
+ * The result resource field should not be accessed from non-Database related 
classes.
+ * It is database class specific and is stored here to associate iterators 
with queries.
+ *
  * @ingroup Database
  */
 class ResultWrapper implements Iterator {
-       /** @var resource */
+       /** @var resource|array|null Optional underlying result handle for 
subclass usage */
        public $result;
 
-       /** @var IDatabase */
+       /** @var IDatabase|null */
        protected $db;
 
        /** @var int */
        protected $pos = 0;
-
-       /** @var object|null */
+       /** @var stdClass|null */
        protected $currentRow = null;
 
        /**
-        * Create a new result object from a result resource and a Database 
object
+        * Create a row iterator from a result resource and an optional 
Database object
         *
-        * @param IDatabase $database
-        * @param resource|ResultWrapper $result
+        * Only Database-related classes should construct ResultWrapper. Other 
code may
+        * use the FakeResultWrapper subclass for convenience or compatibility 
shims, however.
+        *
+        * @param IDatabase|null $db Optional database handle
+        * @param ResultWrapper|array|resource $result Optional underlying 
result handle
         */
-       function __construct( $database, $result ) {
-               $this->db = $database;
-
+       public function __construct( IDatabase $db = null, $result ) {
+               $this->db = $db;
                if ( $result instanceof ResultWrapper ) {
                        $this->result = $result->result;
                } else {
@@ -37,8 +50,8 @@
         *
         * @return int
         */
-       function numRows() {
-               return $this->db->numRows( $this );
+       public function numRows() {
+               return $this->getDB()->numRows( $this );
        }
 
        /**
@@ -49,8 +62,8 @@
         * @return stdClass|bool
         * @throws DBUnexpectedError Thrown if the database returns an error
         */
-       function fetchObject() {
-               return $this->db->fetchObject( $this );
+       public function fetchObject() {
+               return $this->getDB()->fetchObject( $this );
        }
 
        /**
@@ -60,17 +73,8 @@
         * @return array|bool
         * @throws DBUnexpectedError Thrown if the database returns an error
         */
-       function fetchRow() {
-               return $this->db->fetchRow( $this );
-       }
-
-       /**
-        * Free a result object
-        */
-       function free() {
-               $this->db->freeResult( $this );
-               unset( $this->result );
-               unset( $this->db );
+       public function fetchRow() {
+               return $this->getDB()->fetchRow( $this );
        }
 
        /**
@@ -79,19 +83,39 @@
         *
         * @param int $row
         */
-       function seek( $row ) {
-               $this->db->dataSeek( $this, $row );
+       public function seek( $row ) {
+               $this->getDB()->dataSeek( $this, $row );
        }
 
-       /*
-        * ======= Iterator functions =======
-        * Note that using these in combination with the non-iterator functions
-        * above may cause rows to be skipped or repeated.
+       /**
+        * Free a result object
+        *
+        * This either saves memory in PHP (buffered queries) or on the server 
(unbuffered queries).
+        * In general, queries are not large enough in result sets for this to 
be worth calling.
         */
+       public function free() {
+               if ( $this->db ) {
+                       $this->db->freeResult( $this );
+                       $this->db = null;
+               }
+               $this->result = null;
+       }
+
+       /**
+        * @return IDatabase
+        * @throws RuntimeException
+        */
+       private function getDB() {
+               if ( !$this->db ) {
+                       throw new RuntimeException( get_class( $this ) . ' 
needs a DB handle for iteration.' );
+               }
+
+               return $this->db;
+       }
 
        function rewind() {
                if ( $this->numRows() ) {
-                       $this->db->dataSeek( $this, 0 );
+                       $this->getDB()->dataSeek( $this, 0 );
                }
                $this->pos = 0;
                $this->currentRow = null;
@@ -125,9 +149,6 @@
                return $this->currentRow;
        }
 
-       /**
-        * @return bool
-        */
        function valid() {
                return $this->current() !== false;
        }

-- 
To view, visit https://gerrit.wikimedia.org/r/311311
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifcd7163890fa8c7718e88c4244cc38371ed624fc
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to