Jeroen De Dauw has uploaded a new change for review.
https://gerrit.wikimedia.org/r/49707
Change subject: Remove dependency on ORMTable from ORMRow [DO NOT MERGE]
......................................................................
Remove dependency on ORMTable from ORMRow [DO NOT MERGE]
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
---
M includes/db/IORMRow.php
M includes/db/ORMRow.php
M includes/db/ORMTable.php
3 files changed, 269 insertions(+), 155 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/07/49707/1
diff --git a/includes/db/IORMRow.php b/includes/db/IORMRow.php
index 6a7a5bb..f8833f5 100644
--- a/includes/db/IORMRow.php
+++ b/includes/db/IORMRow.php
@@ -33,22 +33,11 @@
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
@@ -75,6 +64,7 @@
* 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
*
@@ -156,6 +146,7 @@
* Load the default values, via getDefaults.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param boolean $override
*/
@@ -168,6 +159,7 @@
* @since 1.20
*
* @param string|null $functionName
+ * @deprecated since 1.21
*
* @return boolean Success indicator
*/
@@ -177,6 +169,7 @@
* Removes the object from the database.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @return boolean Success indicator
*/
@@ -216,9 +209,9 @@
/**
* 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
@@ -240,6 +233,7 @@
* Computes and updates the values of the summary fields.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param array|string|null $summaryFields
*/
@@ -249,6 +243,7 @@
* Sets the value for the @see $updateSummaries field.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param boolean $update
*/
@@ -258,6 +253,7 @@
* Sets the value for the @see $inSummaryMode field.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param boolean $summaryMode
*/
@@ -267,6 +263,7 @@
* Returns the table this IORMRow is a row in.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @return IORMTable
*/
diff --git a/includes/db/ORMRow.php b/includes/db/ORMRow.php
index 6acc124..7966f3c 100644
--- a/includes/db/ORMRow.php
+++ b/includes/db/ORMRow.php
@@ -43,17 +43,12 @@
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,23 +59,29 @@
* 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 ) {
- $this->table = $table;
-
+ public function __construct( IORMTable $table = null, $fields = null,
$loadDefaults = false ) {
if ( !is_array( $fields ) ) {
$fields = array();
}
@@ -96,6 +97,7 @@
* Load the specified fields from the database.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param array|null $fields
* @param boolean $override
@@ -160,6 +162,7 @@
* 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
*
@@ -232,25 +235,10 @@
}
/**
- * 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
*/
@@ -264,10 +252,10 @@
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;
@@ -275,6 +263,22 @@
}
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 );
+ }
+ }
}
/**
@@ -315,6 +319,7 @@
* Load the default values, via getDefaults.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param boolean $override
*/
@@ -327,6 +332,7 @@
* 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
*
@@ -334,9 +340,9 @@
*/
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 );
}
}
@@ -344,6 +350,7 @@
* Updates the object in the database.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param string|null $functionName
*
@@ -355,7 +362,7 @@
$success = $dbw->update(
$this->table->getName(),
$this->getWriteValues(),
- $this->table->getPrefixedValues(
$this->getUpdateConditions() ),
+ $this->table->getPrefixedValues(
$this->getWriteValues() ),
is_null( $functionName ) ? __METHOD__ : $functionName
);
@@ -381,6 +388,7 @@
* Inserts the object into the database.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param string|null $functionName
* @param array|null $options
@@ -413,16 +421,14 @@
* 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();
@@ -435,6 +441,7 @@
* 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
);
@@ -458,6 +465,7 @@
* Can be overridden to get rid of linked data.
*
* @since 1.20
+ * @deprecated since 1.21
*/
protected function onRemoved() {
$this->setField( 'id', null );
@@ -498,55 +506,14 @@
* @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':
- 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;
- }
-
- $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
@@ -554,41 +521,14 @@
* @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
*/
@@ -600,6 +540,7 @@
* Computes and updates the values of the summary fields.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param array|string|null $summaryFields
*/
@@ -611,6 +552,7 @@
* Sets the value for the @see $updateSummaries field.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param boolean $update
*/
@@ -622,6 +564,7 @@
* Sets the value for the @see $inSummaryMode field.
*
* @since 1.20
+ * @deprecated since 1.21
*
* @param boolean $summaryMode
*/
@@ -630,39 +573,10 @@
}
/**
- * 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
*/
diff --git a/includes/db/ORMTable.php b/includes/db/ORMTable.php
index 06f88c1..acf8884 100644
--- a/includes/db/ORMTable.php
+++ b/includes/db/ORMTable.php
@@ -814,10 +814,59 @@
*/
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 @@
*
* @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 @@
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;
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/49707
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I86368821fc2cd0729df5342b8572eb470c0f77a0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
