Remove dependency on ORMTable from ORMRow
authorjeroendedauw <jeroendedauw@gmail.com>
Mon, 18 Feb 2013 19:43:03 +0000 (20:43 +0100)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 10 Apr 2013 08:04:16 +0000 (08:04 +0000)
IORMRow implementing objects take a IORMTable object in their constructor.
The later is a hard to construct service object while the former ideally
should just be a simple wrapper around a database row. This means IORMRow
objects are tightly coupled with IORMTable objects, which makes them
inflexible and makes various things such as testing logic contained in
them needlessly difficult.

This commit gets rid of this nonsense by allowing for construction of
ORMRow objects without providing an ORMTable. All methods dependent on
the table field have been deprecated. Most of these methods have a new
alternative in ORMTable. For instance, saving an ORMRow can now be done
by passing an instance of ORMRow to the updateRow method of an ORMTable
instance, rather then calling save on the ORMRow instance.

Backwards compatibility has been retained except for the fields passed
in the constructor no longer undergoing magical unserialization if it
looks like this is needed. I do not expect this will affect any existing
code though.

Change-Id: I86368821fc2cd0729df5342b8572eb470c0f77a0

includes/db/IORMRow.php
includes/db/ORMRow.php
includes/db/ORMTable.php

index 6bc0cdd..f8833f5 100644 (file)
 
 interface IORMRow {
 
-       /**
-        * Constructor.
-        *
-        * @since 1.20
-        *
-        * @param IORMTable $table
-        * @param array|null $fields
-        * @param boolean $loadDefaults
-        */
-       public function __construct( IORMTable $table, $fields = null, $loadDefaults = false );
-
        /**
         * Load the specified fields from the database.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param array|null $fields
         * @param boolean $override
@@ -74,6 +64,7 @@ interface IORMRow {
         * Gets the value of a field but first loads it if not done so already.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param string$name
         *
@@ -155,6 +146,7 @@ interface IORMRow {
         * Load the default values, via getDefaults.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param boolean $override
         */
@@ -167,6 +159,7 @@ interface IORMRow {
         * @since 1.20
         *
         * @param string|null $functionName
+        * @deprecated since 1.21
         *
         * @return boolean Success indicator
         */
@@ -176,6 +169,7 @@ interface IORMRow {
         * Removes the object from the database.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @return boolean Success indicator
         */
@@ -215,9 +209,9 @@ interface IORMRow {
 
        /**
         * Add an amount (can be negative) to the specified field (needs to be numeric).
-        * TODO: most off this stuff makes more sense in the table class
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param string $field
         * @param integer $amount
@@ -239,6 +233,7 @@ interface IORMRow {
         * Computes and updates the values of the summary fields.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param array|string|null $summaryFields
         */
@@ -248,6 +243,7 @@ interface IORMRow {
         * Sets the value for the @see $updateSummaries field.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param boolean $update
         */
@@ -257,6 +253,7 @@ interface IORMRow {
         * Sets the value for the @see $inSummaryMode field.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param boolean $summaryMode
         */
@@ -266,6 +263,7 @@ interface IORMRow {
         * Returns the table this IORMRow is a row in.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @return IORMTable
         */
index 6c1f27f..ea6ff63 100644 (file)
@@ -42,18 +42,13 @@ class ORMRow implements IORMRow {
         */
        protected $fields = array( 'id' => null );
 
-       /**
-        * @since 1.20
-        * @var ORMTable
-        */
-       protected $table;
-
        /**
         * If the object should update summaries of linked items when changed.
         * For example, update the course_count field in universities when a course in courses is deleted.
         * Settings this to false can prevent needless updating work in situations
         * such as deleting a university, which will then delete all it's courses.
         *
+        * @deprecated since 1.21
         * @since 1.20
         * @var bool
         */
@@ -64,21 +59,29 @@ class ORMRow implements IORMRow {
         * This mode indicates that only summary fields got updated,
         * which allows for optimizations.
         *
+        * @deprecated since 1.21
         * @since 1.20
         * @var bool
         */
        protected $inSummaryMode = false;
 
+       /**
+        * @deprecated since 1.21
+        * @since 1.20
+        * @var ORMTable|null
+        */
+       protected $table;
+
        /**
         * Constructor.
         *
         * @since 1.20
         *
-        * @param IORMTable $table
+        * @param IORMTable|null $table Deprecated since 1.21
         * @param array|null $fields
-        * @param boolean $loadDefaults
+        * @param boolean $loadDefaults Deprecated since 1.21
         */
-       public function __construct( IORMTable $table, $fields = null, $loadDefaults = false ) {
+       public function __construct( IORMTable $table = null, $fields = null, $loadDefaults = false ) {
                $this->table = $table;
 
                if ( !is_array( $fields ) ) {
@@ -96,6 +99,7 @@ class ORMRow implements IORMRow {
         * Load the specified fields from the database.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param array|null $fields
         * @param boolean $override
@@ -160,6 +164,7 @@ class ORMRow implements IORMRow {
         * Gets the value of a field but first loads it if not done so already.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param $name string
         *
@@ -231,26 +236,11 @@ class ORMRow implements IORMRow {
                        && !is_null( $this->getField( 'id' ) );
        }
 
-       /**
-        * Sets multiple fields.
-        *
-        * @since 1.20
-        *
-        * @param array $fields The fields to set
-        * @param boolean $override Override already set fields with the provided values?
-        */
-       public function setFields( array $fields, $override = true ) {
-               foreach ( $fields as $name => $value ) {
-                       if ( $override || !$this->hasField( $name ) ) {
-                               $this->setField( $name, $value );
-                       }
-               }
-       }
-
        /**
         * Gets the fields => values to write to the table.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @return array
         */
@@ -269,10 +259,10 @@ class ORMRow implements IORMRow {
                                switch ( $type ) {
                                        case 'array':
                                                $value = (array)$value;
-                                               // fall-through!
+                                       // fall-through!
                                        case 'blob':
                                                $value = serialize( $value );
-                                               // fall-through!
+                                       // fall-through!
                                }
 
                                $values[$this->table->getPrefixedField( $name )] = $value;
@@ -282,6 +272,22 @@ class ORMRow implements IORMRow {
                return $values;
        }
 
+       /**
+        * Sets multiple fields.
+        *
+        * @since 1.20
+        *
+        * @param array $fields The fields to set
+        * @param boolean $override Override already set fields with the provided values?
+        */
+       public function setFields( array $fields, $override = true ) {
+               foreach ( $fields as $name => $value ) {
+                       if ( $override || !$this->hasField( $name ) ) {
+                               $this->setField( $name, $value );
+                       }
+               }
+       }
+
        /**
         * Serializes the object to an associative array which
         * can then easily be converted into JSON or similar.
@@ -320,6 +326,7 @@ class ORMRow implements IORMRow {
         * Load the default values, via getDefaults.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param boolean $override
         */
@@ -332,6 +339,7 @@ class ORMRow implements IORMRow {
         * when it already exists, or inserting it when it doesn't.
         *
         * @since 1.20
+        * @deprecated since 1.21 Use IORMTable->updateRow or ->insertRow
         *
         * @param string|null $functionName
         *
@@ -339,9 +347,9 @@ class ORMRow implements IORMRow {
         */
        public function save( $functionName = null ) {
                if ( $this->hasIdField() ) {
-                       return $this->saveExisting( $functionName );
+                       return $this->table->updateRow( $this, $functionName );
                } else {
-                       return $this->insert( $functionName );
+                       return $this->table->insertRow( $this, $functionName );
                }
        }
 
@@ -349,6 +357,7 @@ class ORMRow implements IORMRow {
         * Updates the object in the database.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param string|null $functionName
         *
@@ -360,7 +369,7 @@ class ORMRow implements IORMRow {
                $success = $dbw->update(
                        $this->table->getName(),
                        $this->getWriteValues(),
-                       $this->table->getPrefixedValues( $this->getUpdateConditions() ),
+                       $this->table->getPrefixedValues( $this->getWriteValues() ),
                        is_null( $functionName ) ? __METHOD__ : $functionName
                );
 
@@ -386,6 +395,7 @@ class ORMRow implements IORMRow {
         * Inserts the object into the database.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param string|null $functionName
         * @param array|null $options
@@ -418,16 +428,14 @@ class ORMRow implements IORMRow {
         * Removes the object from the database.
         *
         * @since 1.20
+        * @deprecated since 1.21, use IROMtable->removeRow
         *
         * @return boolean Success indicator
         */
        public function remove() {
                $this->beforeRemove();
 
-               $success = $this->table->delete( array( 'id' => $this->getId() ), __METHOD__ );
-
-               // DatabaseBase::delete does not always return true for success as documented...
-               $success = $success !== false;
+               $success = $this->table->removeRow( $this, __METHOD__ );
 
                if ( $success ) {
                        $this->onRemoved();
@@ -440,6 +448,7 @@ class ORMRow implements IORMRow {
         * Gets called before an object is removed from the database.
         *
         * @since 1.20
+        * @deprecated since 1.21
         */
        protected function beforeRemove() {
                $this->loadFields( $this->getBeforeRemoveFields(), false, true );
@@ -463,6 +472,7 @@ class ORMRow implements IORMRow {
         * Can be overridden to get rid of linked data.
         *
         * @since 1.20
+        * @deprecated since 1.21
         */
        protected function onRemoved() {
                $this->setField( 'id', null );
@@ -503,51 +513,14 @@ class ORMRow implements IORMRow {
         * @throws MWException
         */
        public function setField( $name, $value ) {
-               $fields = $this->table->getFields();
-
-               if ( array_key_exists( $name, $fields ) ) {
-                       switch ( $fields[$name] ) {
-                               case 'int':
-                                       $value = (int)$value;
-                                       break;
-                               case 'float':
-                                       $value = (float)$value;
-                                       break;
-                               case 'bool':
-                                       $value = (bool)$value;
-                                       break;
-                               case 'array':
-                                       if ( is_string( $value ) ) {
-                                               $value = unserialize( $value );
-                                       }
-
-                                       if ( !is_array( $value ) ) {
-                                               $value = array();
-                                       }
-                                       break;
-                               case 'blob':
-                                       if ( is_string( $value ) ) {
-                                               $value = unserialize( $value );
-                                       }
-                                       break;
-                               case 'id':
-                                       if ( is_string( $value ) ) {
-                                               $value = (int)$value;
-                                       }
-                                       break;
-                       }
-
-                       $this->fields[$name] = $value;
-               } else {
-                       throw new MWException( 'Attempted to set unknown field ' . $name );
-               }
+               $this->fields[$name] = $value;
        }
 
        /**
         * Add an amount (can be negative) to the specified field (needs to be numeric).
-        * TODO: most off this stuff makes more sense in the table class
         *
         * @since 1.20
+        * @deprecated since 1.21, use IORMTable->addToField
         *
         * @param string $field
         * @param integer $amount
@@ -555,41 +528,14 @@ class ORMRow implements IORMRow {
         * @return boolean Success indicator
         */
        public function addToField( $field, $amount ) {
-               if ( $amount == 0 ) {
-                       return true;
-               }
-
-               if ( !$this->hasIdField() ) {
-                       return false;
-               }
-
-               $absoluteAmount = abs( $amount );
-               $isNegative = $amount < 0;
-
-               $dbw = $this->table->getWriteDbConnection();
-
-               $fullField = $this->table->getPrefixedField( $field );
-
-               $success = $dbw->update(
-                       $this->table->getName(),
-                       array( "$fullField=$fullField" . ( $isNegative ? '-' : '+' ) . $absoluteAmount ),
-                       array( $this->table->getPrefixedField( 'id' ) => $this->getId() ),
-                       __METHOD__
-               );
-
-               if ( $success && $this->hasField( $field ) ) {
-                       $this->setField( $field, $this->getField( $field ) + $amount );
-               }
-
-               $this->table->releaseConnection( $dbw );
-
-               return $success;
+               return $this->table->addToField( $this->getUpdateConditions(), $field, $amount );
        }
 
        /**
         * Return the names of the fields.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @return array
         */
@@ -601,6 +547,7 @@ class ORMRow implements IORMRow {
         * Computes and updates the values of the summary fields.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param array|string|null $summaryFields
         */
@@ -612,6 +559,7 @@ class ORMRow implements IORMRow {
         * Sets the value for the @see $updateSummaries field.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param boolean $update
         */
@@ -623,6 +571,7 @@ class ORMRow implements IORMRow {
         * Sets the value for the @see $inSummaryMode field.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @param boolean $summaryMode
         */
@@ -630,40 +579,11 @@ class ORMRow implements IORMRow {
                $this->inSummaryMode = $summaryMode;
        }
 
-       /**
-        * Return if any fields got changed.
-        *
-        * @since 1.20
-        *
-        * @param IORMRow $object
-        * @param boolean|array $excludeSummaryFields
-        *  When set to true, summary field changes are ignored.
-        *  Can also be an array of fields to ignore.
-        *
-        * @return boolean
-        */
-       protected function fieldsChanged( IORMRow $object, $excludeSummaryFields = false ) {
-               $exclusionFields = array();
-
-               if ( $excludeSummaryFields !== false ) {
-                       $exclusionFields = is_array( $excludeSummaryFields ) ? $excludeSummaryFields : $this->table->getSummaryFields();
-               }
-
-               foreach ( $this->fields as $name => $value ) {
-                       $excluded = $excludeSummaryFields && in_array( $name, $exclusionFields );
-
-                       if ( !$excluded && $object->getField( $name ) !== $value ) {
-                               return true;
-                       }
-               }
-
-               return false;
-       }
-
        /**
         * Returns the table this IORMRow is a row in.
         *
         * @since 1.20
+        * @deprecated since 1.21
         *
         * @return IORMTable
         */
index bcbe94a..3a432cd 100644 (file)
@@ -814,10 +814,59 @@ class ORMTable extends DBAccessBase implements IORMTable {
         */
        public function getFieldsFromDBResult( stdClass $result ) {
                $result = (array)$result;
-               return array_combine(
+
+               $rawFields = array_combine(
                        $this->unprefixFieldNames( array_keys( $result ) ),
                        array_values( $result )
                );
+
+               $fieldDefinitions = $this->getFields();
+               $fields = array();
+
+               foreach ( $rawFields as $name => $value ) {
+                       if ( array_key_exists( $name, $fieldDefinitions ) ) {
+                               switch ( $fieldDefinitions[$name] ) {
+                                       case 'int':
+                                               $value = (int)$value;
+                                               break;
+                                       case 'float':
+                                               $value = (float)$value;
+                                               break;
+                                       case 'bool':
+                                               if ( is_string( $value ) ) {
+                                                       $value = $value !== '0';
+                                               } elseif ( is_int( $value ) ) {
+                                                       $value = $value !== 0;
+                                               }
+                                               break;
+                                       case 'array':
+                                               if ( is_string( $value ) ) {
+                                                       $value = unserialize( $value );
+                                               }
+
+                                               if ( !is_array( $value ) ) {
+                                                       $value = array();
+                                               }
+                                               break;
+                                       case 'blob':
+                                               if ( is_string( $value ) ) {
+                                                       $value = unserialize( $value );
+                                               }
+                                               break;
+                                       case 'id':
+                                               if ( is_string( $value ) ) {
+                                                       $value = (int)$value;
+                                               }
+                                               break;
+                               }
+
+                               $fields[$name] = $value;
+                       } else {
+                               throw new MWException( 'Attempted to set unknown field ' . $name );
+                       }
+               }
+
+               return $fields;
        }
 
        /**
@@ -867,14 +916,15 @@ class ORMTable extends DBAccessBase implements IORMTable {
         *
         * @since 1.20
         *
-        * @param array $data
+        * @param array $fields
         * @param boolean $loadDefaults
         *
         * @return IORMRow
         */
-       public function newRow( array $data, $loadDefaults = false ) {
+       public function newRow( array $fields, $loadDefaults = false ) {
                $class = $this->getRowClass();
-               return new $class( $this, $data, $loadDefaults );
+
+               return new $class( $this, $fields, $loadDefaults );
        }
 
        /**
@@ -901,4 +951,157 @@ class ORMTable extends DBAccessBase implements IORMTable {
                return array_key_exists( $name, $this->getFields() );
        }
 
+       /**
+        * Updated the provided row in the database.
+        *
+        * @since 1.21
+        *
+        * @param IORMRow $row The row to save
+        * @param string|null $functionName
+        *
+        * @return boolean Success indicator
+        */
+       public function updateRow( IORMRow $row, $functionName = null ) {
+               $dbw = $this->getWriteDbConnection();
+
+               $success = $dbw->update(
+                       $this->getName(),
+                       $this->getWriteValues( $row ),
+                       $this->getPrefixedValues( array( 'id' => $row->getId() ) ),
+                       is_null( $functionName ) ? __METHOD__ : $functionName
+               );
+
+               $this->releaseConnection( $dbw );
+
+               // DatabaseBase::update does not always return true for success as documented...
+               return $success !== false;
+       }
+
+       /**
+        * Inserts the provided row into the database.
+        *
+        * @since 1.21
+        *
+        * @param IORMRow $row
+        * @param string|null $functionName
+        * @param array|null $options
+        *
+        * @return boolean Success indicator
+        */
+       public function insertRow( IORMRow $row, $functionName = null, array $options = null ) {
+               $dbw = $this->getWriteDbConnection();
+
+               $success = $dbw->insert(
+                       $this->getName(),
+                       $this->getWriteValues( $row ),
+                       is_null( $functionName ) ? __METHOD__ : $functionName,
+                       $options
+               );
+
+               // DatabaseBase::insert does not always return true for success as documented...
+               $success = $success !== false;
+
+               if ( $success ) {
+                       $row->setField( 'id', $dbw->insertId() );
+               }
+
+               $this->releaseConnection( $dbw );
+
+               return $success;
+       }
+
+       /**
+        * Gets the fields => values to write to the table.
+        *
+        * @since 1.20
+        *
+        * @param IORMRow $row
+        *
+        * @return array
+        */
+       protected function getWriteValues( IORMRow $row ) {
+               $values = array();
+
+               $rowFields = $row->getFields();
+
+               foreach ( $this->getFields() as $name => $type ) {
+                       if ( array_key_exists( $name, $rowFields ) ) {
+                               $value = $rowFields[$name];
+
+                               switch ( $type ) {
+                                       case 'array':
+                                               $value = (array)$value;
+                                       // fall-through!
+                                       case 'blob':
+                                               $value = serialize( $value );
+                                       // fall-through!
+                               }
+
+                               $values[$this->getPrefixedField( $name )] = $value;
+                       }
+               }
+
+               return $values;
+       }
+
+       /**
+        * Removes the provided row from the database.
+        *
+        * @since 1.21
+        *
+        * @param IORMRow $row
+        * @param string|null $functionName
+        *
+        * @return boolean Success indicator
+        */
+       public function removeRow( IORMRow $row, $functionName = null ) {
+               $success = $this->delete(
+                       array( 'id' => $row->getId() ),
+                       is_null( $functionName ) ? __METHOD__ : $functionName
+               );
+
+               // DatabaseBase::delete does not always return true for success as documented...
+               return $success !== false;
+       }
+
+       /**
+        * Add an amount (can be negative) to the specified field (needs to be numeric).
+        *
+        * @since 1.21
+        *
+        * @param array $conditions
+        * @param string $field
+        * @param integer $amount
+        *
+        * @return boolean Success indicator
+        * @throws MWException
+        */
+       public function addToField( array $conditions, $field, $amount ) {
+               if ( !array_key_exists( $field, $this->fields ) ) {
+                       throw new MWException( 'Unknown field "' . $field . '" provided' );
+               }
+
+               if ( $amount == 0 ) {
+                       return true;
+               }
+
+               $absoluteAmount = abs( $amount );
+               $isNegative = $amount < 0;
+
+               $fullField = $this->getPrefixedField( $field );
+
+               $dbw = $this->getWriteDbConnection();
+
+               $success = $dbw->update(
+                       $this->getName(),
+                       array( "$fullField=$fullField" . ( $isNegative ? '-' : '+' ) . $absoluteAmount ),
+                       $this->getPrefixedValues( $conditions ),
+                       __METHOD__
+               ) !== false; // DatabaseBase::update does not always return true for success as documented...
+
+               $this->releaseConnection( $dbw );
+
+               return $success;
+       }
+
 }