1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 09:18:48 +02:00

Remove partial objects from Lisk

Summary:
Ref T4420. This was a performance hack introduced long ago to make typeaheads for users a little cheaper. The idea was that you could load some of an object's columns and skip other ones.

We now always load users on demand, so the cost of loading the whole objects is very small. No other use cases ever arose for this, and it seems unlikely that they will in the future. Remove it all.

Test Plan:
- Grepped for `CONFIG_PARTIAL_OBJECTS`.
- Grepped for `dirtyFields`.
- Grepped for `missingFields`.
- Grepped for `resetDirtyFields`.
- Grepped for `loadColumns`.
- Grepped for `loadColumnsWhere`.
- Grepped for `loadRawDataWhere`.
- Loaded and saved some lisk objects.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4420

Differential Revision: https://secure.phabricator.com/D9895
This commit is contained in:
epriestley 2014-07-17 15:49:21 -07:00
parent b8d604acaf
commit c52b3c28e1
2 changed files with 3 additions and 112 deletions

View file

@ -113,7 +113,6 @@ final class PhabricatorUser
public function getConfiguration() { public function getConfiguration() {
return array( return array(
self::CONFIG_AUX_PHID => true, self::CONFIG_AUX_PHID => true,
self::CONFIG_PARTIAL_OBJECTS => true,
) + parent::getConfiguration(); ) + parent::getConfiguration();
} }

View file

@ -168,7 +168,6 @@ abstract class LiskDAO {
const CONFIG_TIMESTAMPS = 'timestamps'; const CONFIG_TIMESTAMPS = 'timestamps';
const CONFIG_AUX_PHID = 'auxiliary-phid'; const CONFIG_AUX_PHID = 'auxiliary-phid';
const CONFIG_SERIALIZATION = 'col-serialization'; const CONFIG_SERIALIZATION = 'col-serialization';
const CONFIG_PARTIAL_OBJECTS = 'partial-objects';
const CONFIG_BINARY = 'binary'; const CONFIG_BINARY = 'binary';
const SERIALIZATION_NONE = 'id'; const SERIALIZATION_NONE = 'id';
@ -181,8 +180,6 @@ abstract class LiskDAO {
const COUNTER_TABLE_NAME = 'lisk_counter'; const COUNTER_TABLE_NAME = 'lisk_counter';
private $dirtyFields = array();
private $missingFields = array();
private static $processIsolationLevel = 0; private static $processIsolationLevel = 0;
private static $transactionIsolationLevel = 0; private static $transactionIsolationLevel = 0;
@ -207,10 +204,6 @@ abstract class LiskDAO {
if ($id_key) { if ($id_key) {
$this->$id_key = null; $this->$id_key = null;
} }
if ($this->getConfigOption(self::CONFIG_PARTIAL_OBJECTS)) {
$this->resetDirtyFields();
}
} }
@ -345,16 +338,6 @@ abstract class LiskDAO {
* This will cause Lisk to JSON-serialize the 'complex' field before it is * This will cause Lisk to JSON-serialize the 'complex' field before it is
* written, and unserialize it when it is read. * written, and unserialize it when it is read.
* *
* CONFIG_PARTIAL_OBJECTS
* Sometimes, it is useful to load only some fields of an object (such as
* when you are loading all objects of a class, but only care about a few
* fields). Turning on this option (by setting it to a truthy value) allows
* users of the class to create/use partial objects, but it comes with some
* side effects: your class cannot override the setters and getters provided
* by Lisk (use readField and writeField instead), and you should not
* directly access or assign protected members of your class (use the getters
* and setters).
*
* CONFIG_BINARY * CONFIG_BINARY
* You can optionally provide a map of columns to a flag indicating that * You can optionally provide a map of columns to a flag indicating that
* they store binary data. These columns will not raise an error when * they store binary data. These columns will not raise an error when
@ -368,7 +351,6 @@ abstract class LiskDAO {
return array( return array(
self::CONFIG_IDS => self::IDS_AUTOINCREMENT, self::CONFIG_IDS => self::IDS_AUTOINCREMENT,
self::CONFIG_TIMESTAMPS => true, self::CONFIG_TIMESTAMPS => true,
self::CONFIG_PARTIAL_OBJECTS => false,
); );
} }
@ -435,18 +417,6 @@ abstract class LiskDAO {
return $this->loadAllWhere('1 = 1'); return $this->loadAllWhere('1 = 1');
} }
/**
* Loads all objects, but only fetches the specified columns.
*
* @param array Array of canonical column names as strings
* @return dict Dictionary of all objects, keyed by ID.
*
* @task load
*/
public function loadColumns(array $columns) {
return $this->loadColumnsWhere($columns, '1 = 1');
}
/** /**
* Load all objects which match a WHERE clause. You provide everything after * Load all objects which match a WHERE clause. You provide everything after
@ -463,30 +433,6 @@ abstract class LiskDAO {
* @task load * @task load
*/ */
public function loadAllWhere($pattern /* , $arg, $arg, $arg ... */) { public function loadAllWhere($pattern /* , $arg, $arg, $arg ... */) {
$args = func_get_args();
array_unshift($args, null);
$data = call_user_func_array(
array($this, 'loadRawDataWhere'),
$args);
return $this->loadAllFromArray($data);
}
/**
* Loads selected columns from objects that match a WHERE clause. You must
* provide everything after the WHERE. See loadAllWhere().
*
* @param array List of column names.
* @param string queryfx()-style SQL WHERE clause.
* @param ... Zero or more conversions.
* @return dict Dictionary of matching objecks, keyed by ID.
*
* @task load
*/
public function loadColumnsWhere(array $columns, $pattern /* , $args... */) {
if (!$this->getConfigOption(self::CONFIG_PARTIAL_OBJECTS)) {
throw new BadMethodCallException(
'This class does not support partial objects.');
}
$args = func_get_args(); $args = func_get_args();
$data = call_user_func_array( $data = call_user_func_array(
array($this, 'loadRawDataWhere'), array($this, 'loadRawDataWhere'),
@ -509,7 +455,6 @@ abstract class LiskDAO {
*/ */
public function loadOneWhere($pattern /* , $arg, $arg, $arg ... */) { public function loadOneWhere($pattern /* , $arg, $arg, $arg ... */) {
$args = func_get_args(); $args = func_get_args();
array_unshift($args, null);
$data = call_user_func_array( $data = call_user_func_array(
array($this, 'loadRawDataWhere'), array($this, 'loadRawDataWhere'),
$args); $args);
@ -528,7 +473,7 @@ abstract class LiskDAO {
} }
protected function loadRawDataWhere($columns, $pattern /* , $args... */) { protected function loadRawDataWhere($pattern /* , $args... */) {
$connection = $this->establishConnection('r'); $connection = $this->establishConnection('r');
$lock_clause = ''; $lock_clause = '';
@ -539,25 +484,10 @@ abstract class LiskDAO {
} }
$args = func_get_args(); $args = func_get_args();
$args = array_slice($args, 2); $args = array_slice($args, 1);
if (!$columns) { $pattern = 'SELECT * FROM %T WHERE '.$pattern.' %Q';
$column = '*';
} else {
$column = '%LC';
$columns[] = $this->getIDKey();
$properties = $this->getProperties();
$this->missingFields = array_diff_key(
array_flip($properties),
array_flip($columns));
}
$pattern = 'SELECT '.$column.' FROM %T WHERE '.$pattern.' %Q';
array_unshift($args, $this->getTableName()); array_unshift($args, $this->getTableName());
if ($columns) {
array_unshift($args, $columns);
}
array_push($args, $lock_clause); array_push($args, $lock_clause);
array_unshift($args, $pattern); array_unshift($args, $pattern);
@ -1141,9 +1071,6 @@ abstract class LiskDAO {
$this->willSaveObject(); $this->willSaveObject();
$data = $this->getPropertyValues(); $data = $this->getPropertyValues();
if ($this->getConfigOption(self::CONFIG_PARTIAL_OBJECTS)) {
$data = array_intersect_key($data, $this->dirtyFields);
}
$this->willWriteData($data); $this->willWriteData($data);
$map = array(); $map = array();
@ -1176,10 +1103,6 @@ abstract class LiskDAO {
$this->didWriteData(); $this->didWriteData();
if ($this->getConfigOption(self::CONFIG_PARTIAL_OBJECTS)) {
$this->resetDirtyFields();
}
return $this; return $this;
} }
@ -1286,10 +1209,6 @@ abstract class LiskDAO {
$this->didWriteData(); $this->didWriteData();
if ($this->getConfigOption(self::CONFIG_PARTIAL_OBJECTS)) {
$this->resetDirtyFields();
}
return $this; return $this;
} }
@ -1680,20 +1599,6 @@ abstract class LiskDAO {
} }
} }
/**
* Resets the dirty fields (fields which need to be written on next save/
* update/insert/replace). If this DAO has timestamps, the modified time
* is always a dirty field.
*
* @task util
*/
private function resetDirtyFields() {
$this->dirtyFields = array();
if ($this->getConfigOption(self::CONFIG_TIMESTAMPS)) {
$this->dirtyFields['dateModified'] = true;
}
}
/** /**
* Black magic. Builds implied get*() and set*() for all properties. * Black magic. Builds implied get*() and set*() for all properties.
* *
@ -1719,10 +1624,6 @@ abstract class LiskDAO {
// optimizations. // optimizations.
static $dispatch_map = array(); static $dispatch_map = array();
static $partial = null;
if ($partial === null) {
$partial = $this->getConfigOption(self::CONFIG_PARTIAL_OBJECTS);
}
if ($method[0] === 'g') { if ($method[0] === 'g') {
if (isset($dispatch_map[$method])) { if (isset($dispatch_map[$method])) {
@ -1738,10 +1639,6 @@ abstract class LiskDAO {
$dispatch_map[$method] = $property; $dispatch_map[$method] = $property;
} }
if ($partial && isset($this->missingFields[$property])) {
throw new Exception("Cannot get field that wasn't loaded: {$property}");
}
return $this->readField($property); return $this->readField($property);
} }
@ -1759,11 +1656,6 @@ abstract class LiskDAO {
} }
$dispatch_map[$method] = $property; $dispatch_map[$method] = $property;
} }
if ($partial) {
// Accept writes to fields that weren't initially loaded
unset($this->missingFields[$property]);
$this->dirtyFields[$property] = true;
}
$this->writeField($property, $args[0]); $this->writeField($property, $args[0]);