revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 01:08:50 +02:00
Code Issues Releases Wiki Activity

Don't special-case LiskDAO->load(0)

Browse source 
Summary:
Lisk currently behaves in two different ways if you call it like `load("cow")` (throws) versus `load(99999999)` (returns null), where neither ID exists.

This was intended to catch programming errors as distinct from missing data, but in practice the former is very rare and you have to handle the latter in most cases anyway. The case where you pass "0" is particularly confusing. See D2971 for an example.

On the balance, I think this ends up being far more confusing than helpful. Instead, just return NULL if we're sure there's no such object.

Test Plan: Reasoned about program behavior.

Reviewers: alanh, btrahan, vrana

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2977
This commit is contained in:
epriestley 2012-07-16 09:50:23 -07:00
parent 8fd97f4352
commit cb8120551f
2 changed files with 19 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -442,8 +442,8 @@ abstract class LiskDAO {
* @task load
*/
public function load($id) {
if (!($id = (int)$id)) {
throw new Exception("Bogus ID provided to load().");
if (!$id || (!is_int($id) && !ctype_digit($id))) {
return null;
}
return $this->loadOneWhere(

17
src/infrastructure/storage/lisk/__tests__/LiskFixtureTestCase.php
View file

@ -93,6 +93,23 @@ final class LiskFixtureTestCase extends PhabricatorTestCase {
}
}
public function testGarbageLoadCalls() {
$obj = new HarbormasterObject();
$obj->save();
$id = $obj->getID();
$load = new HarbormasterObject();
$this->assertEqual(null, $load->load(0));
$this->assertEqual(null, $load->load(-1));
$this->assertEqual(null, $load->load(9999));
$this->assertEqual(null, $load->load(''));
$this->assertEqual(null, $load->load('cow'));
$this->assertEqual(null, $load->load($id."cow"));
$this->assertEqual(true, (bool)$load->load((int)$id));
$this->assertEqual(true, (bool)$load->load((string)$id));
}
}