revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 16:52:41 +01:00
Code Issues Releases Wiki Activity

Completely remove "LiskDAOSet" and "loadRelatives/loadOneRelative"

Browse source 
Summary: Fixes T13218. We have no more callers to any of this and can get rid of it forever.

Test Plan: Grepped for all four API methods, `LiskDAOSet`, and `inSet`.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13218

Differential Revision: https://secure.phabricator.com/D19879
This commit is contained in:
epriestley 2018-12-12 16:15:19 -08:00
parent 02933acbd5
commit 9aa5a52fbd
3 changed files with 0 additions and 273 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/__phutil_library_map__.php
View file

@ -1633,7 +1633,6 @@ phutil_register_library_map(array(
'LegalpadTransactionView' => 'applications/legalpad/view/LegalpadTransactionView.php',
'LiskChunkTestCase' => 'infrastructure/storage/lisk/__tests__/LiskChunkTestCase.php',
'LiskDAO' => 'infrastructure/storage/lisk/LiskDAO.php',
'LiskDAOSet' => 'infrastructure/storage/lisk/LiskDAOSet.php',
'LiskDAOTestCase' => 'infrastructure/storage/lisk/__tests__/LiskDAOTestCase.php',
'LiskEphemeralObjectException' => 'infrastructure/storage/lisk/LiskEphemeralObjectException.php',
'LiskFixtureTestCase' => 'infrastructure/storage/lisk/__tests__/LiskFixtureTestCase.php',
@ -7200,7 +7199,6 @@ phutil_register_library_map(array(
'Phobject',
'AphrontDatabaseTableRefInterface',
),
'LiskDAOSet' => 'Phobject',
'LiskDAOTestCase' => 'PhabricatorTestCase',
'LiskEphemeralObjectException' => 'Exception',
'LiskFixtureTestCase' => 'PhabricatorTestCase',

170
src/infrastructure/storage/lisk/LiskDAO.php
View file

@ -193,8 +193,6 @@ abstract class LiskDAO extends Phobject
private static $connections = array();
private $inSet = null;
protected $id;
protected $phid;
protected $dateCreated;
@ -659,179 +657,11 @@ abstract class LiskDAO extends Phobject
} else {
$result[] = $obj->loadFromArray($row);
}
if ($this->inSet) {
$this->inSet->addToSet($obj);
}
}
return $result;
}
/**
* This method helps to prevent the 1+N queries problem. It happens when you
* execute a query for each row in a result set. Like in this code:
*
* COUNTEREXAMPLE, name=Easy to write but expensive to execute
* $diffs = id(new DifferentialDiff())->loadAllWhere(
* 'revisionID = %d',
* $revision->getID());
* foreach ($diffs as $diff) {
* $changesets = id(new DifferentialChangeset())->loadAllWhere(
* 'diffID = %d',
* $diff->getID());
* // Do something with $changesets.
* }
*
* One can solve this problem by reading all the dependent objects at once and
* assigning them later:
*
* COUNTEREXAMPLE, name=Cheaper to execute but harder to write and maintain
* $diffs = id(new DifferentialDiff())->loadAllWhere(
* 'revisionID = %d',
* $revision->getID());
* $all_changesets = id(new DifferentialChangeset())->loadAllWhere(
* 'diffID IN (%Ld)',
* mpull($diffs, 'getID'));
* $all_changesets = mgroup($all_changesets, 'getDiffID');
* foreach ($diffs as $diff) {
* $changesets = idx($all_changesets, $diff->getID(), array());
* // Do something with $changesets.
* }
*
* The method @{method:loadRelatives} abstracts this approach which allows
* writing a code which is simple and efficient at the same time:
*
* name=Easy to write and cheap to execute
* $diffs = $revision->loadRelatives(new DifferentialDiff(), 'revisionID');
* foreach ($diffs as $diff) {
* $changesets = $diff->loadRelatives(
* new DifferentialChangeset(),
* 'diffID');
* // Do something with $changesets.
* }
*
* This will load dependent objects for all diffs in the first call of
* @{method:loadRelatives} and use this result for all following calls.
*
* The method supports working with set of sets, like in this code:
*
* $diffs = $revision->loadRelatives(new DifferentialDiff(), 'revisionID');
* foreach ($diffs as $diff) {
* $changesets = $diff->loadRelatives(
* new DifferentialChangeset(),
* 'diffID');
* foreach ($changesets as $changeset) {
* $hunks = $changeset->loadRelatives(
* new DifferentialHunk(),
* 'changesetID');
* // Do something with hunks.
* }
* }
*
* This code will execute just three queries - one to load all diffs, one to
* load all their related changesets and one to load all their related hunks.
* You can try to write an equivalent code without using this method as
* a homework.
*
* The method also supports retrieving referenced objects, for example authors
* of all diffs (using shortcut @{method:loadOneRelative}):
*
* foreach ($diffs as $diff) {
* $author = $diff->loadOneRelative(
* new PhabricatorUser(),
* 'phid',
* 'getAuthorPHID');
* // Do something with author.
* }
*
* It is also possible to specify additional conditions for the `WHERE`
* clause. Similarly to @{method:loadAllWhere}, you can specify everything
* after `WHERE` (except `LIMIT`). Contrary to @{method:loadAllWhere}, it is
* allowed to pass only a constant string (`%` doesn't have a special
* meaning). This is intentional to avoid mistakes with using data from one
* row in retrieving other rows. Example of a correct usage:
*
* $status = $author->loadOneRelative(
* new PhabricatorCalendarEvent(),
* 'userPHID',
* 'getPHID',
* '(UNIX_TIMESTAMP() BETWEEN dateFrom AND dateTo)');
*
* @param LiskDAO Type of objects to load.
* @param string Name of the column in target table.
* @param string Method name in this table.
* @param string Additional constraints on returned rows. It supports no
* placeholders and requires putting the WHERE part into
* parentheses. It's not possible to use LIMIT.
* @return list Objects of type $object.
*
* @task load
*/
public function loadRelatives(
LiskDAO $object,
$foreign_column,
$key_method = 'getID',
$where = '') {
if (!$this->inSet) {
id(new LiskDAOSet())->addToSet($this);
}
$relatives = $this->inSet->loadRelatives(
$object,
$foreign_column,
$key_method,
$where);
return idx($relatives, $this->$key_method(), array());
}
/**
* Load referenced row. See @{method:loadRelatives} for details.
*
* @param LiskDAO Type of objects to load.
* @param string Name of the column in target table.
* @param string Method name in this table.
* @param string Additional constraints on returned rows. It supports no
* placeholders and requires putting the WHERE part into
* parentheses. It's not possible to use LIMIT.
* @return LiskDAO Object of type $object or null if there's no such object.
*
* @task load
*/
final public function loadOneRelative(
LiskDAO $object,
$foreign_column,
$key_method = 'getID',
$where = '') {
$relatives = $this->loadRelatives(
$object,
$foreign_column,
$key_method,
$where);
if (!$relatives) {
return null;
}
if (count($relatives) > 1) {
throw new AphrontCountQueryException(
pht(
'More than one result from %s!',
__FUNCTION__.'()'));
}
return reset($relatives);
}
final public function putInSet(LiskDAOSet $set) {
$this->inSet = $set;
return $this;
}
final protected function getInSet() {
return $this->inSet;
}
/* -( Examining Objects )-------------------------------------------------- */

101
src/infrastructure/storage/lisk/LiskDAOSet.php
View file

@ -1,101 +0,0 @@
<?php
/**
* You usually don't need to use this class directly as it is controlled by
* @{class:LiskDAO}. You can create it if you want to work with objects of same
* type from different sources as with one set. Let's say you want to get
* e-mails of all users involved in a revision:
*
* $users = new LiskDAOSet();
* $users->addToSet($author);
* foreach ($reviewers as $reviewer) {
* $users->addToSet($reviewer);
* }
* foreach ($ccs as $cc) {
* $users->addToSet($cc);
* }
* // Preload e-mails of all involved users and return e-mails of author.
* $author_emails = $author->loadRelatives(
* new PhabricatorUserEmail(),
* 'userPHID',
* 'getPHID');
*/
final class LiskDAOSet extends Phobject {
private $daos = array();
private $relatives = array();
private $subsets = array();
public function addToSet(LiskDAO $dao) {
if ($this->relatives) {
throw new Exception(
pht(
"Don't call %s after loading data!",
__FUNCTION__.'()'));
}
$this->daos[] = $dao;
$dao->putInSet($this);
return $this;
}
/**
* The main purpose of this method is to break cyclic dependency.
* It removes all objects from this set and all subsets created by it.
*/
public function clearSet() {
$this->daos = array();
$this->relatives = array();
foreach ($this->subsets as $set) {
$set->clearSet();
}
$this->subsets = array();
return $this;
}
/**
* See @{method:LiskDAO::loadRelatives}.
*/
public function loadRelatives(
LiskDAO $object,
$foreign_column,
$key_method = 'getID',
$where = '') {
$relatives = &$this->relatives[
get_class($object)."-{$foreign_column}-{$key_method}-{$where}"];
if ($relatives === null) {
$ids = array();
foreach ($this->daos as $dao) {
$id = $dao->$key_method();
if ($id !== null) {
$ids[$id] = $id;
}
}
if (!$ids) {
$relatives = array();
} else {
$set = new LiskDAOSet();
$this->subsets[] = $set;
$conn = $object->establishConnection('r');
if (strlen($where)) {
$where_clause = qsprintf($conn, 'AND %Q', $where);
} else {
$where_clause = qsprintf($conn, '');
}
$relatives = $object->putInSet($set)->loadAllWhere(
'%C IN (%Ls) %Q',
$foreign_column,
$ids,
$where_clause);
$relatives = mgroup($relatives, 'get'.$foreign_column);
}
}
return $relatives;
}
}