Skip to content

Commit 0a17877

Browse files
driusandriusan
authored
[NDB_Factory] Remove testing mode (#7199)
The "Testing" mode in NDB_Factory complicates our code and frequently confuses phan. It appears to not be used by our testing suite, and the test suite often explicitly sets it to not-test mode in order to get real connections, and uses setDatabase/setConfig/etc to inject mocks rather than using the factory's internal test mode. This removes the mode and simplifies our code.
1 parent 0907364 commit 0a17877

13 files changed

+93
-154
lines changed

‎php/libraries/NDB_Client.class.inc‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,11 @@ class NDB_Client
6565
$DBSettings['host'],
6666
true
6767
);
68+
// Now make sure the factory reference is the same as the
69+
// singleton reference
70+
$factory->setDatabase($DB);
6871
}
6972

70-
// Now make sure the factory reference is the same as the
71-
// singleton reference
7273
$DB = $factory->database();
7374

7475
// add extra include paths. This must be done

‎php/libraries/NDB_Factory.class.inc‎

Lines changed: 33 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
* are usually singletons. Instead of directly calling class::singleton staticly,
55
* this factory should be used so that a mock class can be subbed in for testing.
66
*
7-
* If the Factory is in testing mode (setTesting(true) was called), a mock will
8-
* be returned. Otherwise, the normal NDB_ prefixed object will be returned.
7+
* Mocks are injected using the setDatabase/setConfig/etc methods.
98
*
109
* PHP Version 7
1110
*
@@ -25,11 +24,6 @@ use \LORIS\StudyEntities\Candidate\CandID;
2524
* @author Dave MacFarlane <david.macfarlane2@mcgill.ca>
2625
* @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
2726
* @link https://www.github.com/aces/Loris/
28-
*
29-
* Phan currently has bad support for PHPUnit's Mock objects which is
30-
* creating false positive with the below rule.
31-
*
32-
* @phan-file-suppress PhanUndeclaredClassMethod
3327
*/
3428
class NDB_Factory
3529
{
@@ -43,9 +37,6 @@ class NDB_Factory
4337
private static $_timepoints = [];
4438

4539
private $_baseURL = "";
46-
var $Testing; // Whether or not Mock objects should be returned instead of
47-
// real ones
48-
//
4940

5041
/**
5142
* Settings object
@@ -55,18 +46,6 @@ class NDB_Factory
5546
*/
5647
private static $_settings = null;
5748

58-
/**
59-
* Sets whether the factory should return real objects or testing objects
60-
*
61-
* @param boolean $val Whether testing should be enabled
62-
*
63-
* @return void
64-
*/
65-
function setTesting(bool $val): void
66-
{
67-
$this->Testing = $val;
68-
}
69-
7049
/**
7150
* Returns a single factory object. This must be used instead of being
7251
* constructed directly so that the testing suite and Loris code are
@@ -117,34 +96,14 @@ class NDB_Factory
11796
*/
11897
function config(?string $configfile = '../project/config.xml'): \NDB_Config
11998
{
120-
if (self::$config !== null) {
121-
// The below suppression is necessary to satisfy phan. It flags an
122-
// error here because the function is declared to return
123-
// NDB_Config and phan doesn't understand MockNDB_Config (as it is
124-
// generated dynamically by PHPUnit).
125-
// We know this will always return \NDB_Config since this is guaranteed
126-
// by the return type of the function.
127-
//
128-
// @phan-suppress-next-line PhanTypeMismatchReturn
129-
return self::$config;
130-
}
131-
if ($this->Testing) {
132-
$config = new MockNDB_Config();
133-
$configfile = '../test/config.xml';
134-
} else {
135-
$config = NDB_Config::singleton($configfile);
99+
$config = self::$config;
100+
if ($config !== null) {
101+
return $config;
136102
}
137103

138-
self::$config = $config;
104+
$config = NDB_Config::singleton($configfile);
139105

140-
// The below suppression is necessary to satisfy phan. It flags an
141-
// error here because the function is declared to return
142-
// NDB_Config and phan doesn't understand MockNDB_Config (as it is
143-
// generated dynamically by PHPUnit).
144-
// We know this will always return \NDB_Config since this is guaranteed
145-
// by the return type of the function.
146-
//
147-
// @phan-suppress-next-line PhanTypeMismatchReturn
106+
self::$config = $config;
148107
return $config;
149108
}
150109

@@ -169,22 +128,11 @@ class NDB_Factory
169128
*/
170129
function user(): \User
171130
{
172-
if (self::$_user !== null) {
173-
// The below suppression is necessary to satisfy phan. It flags an
174-
// error here because the function is declared to return
175-
// User and phan doesn't understand MockUser (as it is
176-
// generated dynamically by PHPUnit).
177-
// We know this will always return \User since this is guaranteed
178-
// by the return type of the function.
179-
// @phan-suppress-next-line PhanTypeMismatchReturn
180-
return self::$_user;
181-
}
182-
if ($this->Testing) {
183-
Mock::generate("User");
184-
$user = new MockUser();
185-
} else {
186-
$user = \User::singleton();
131+
$user = self::$_user;
132+
if ($user !== null) {
133+
return $user;
187134
}
135+
$user = \User::singleton();
188136
self::$_user = $user;
189137
return $user;
190138
}
@@ -210,21 +158,11 @@ class NDB_Factory
210158
*/
211159
function database(): \Database
212160
{
213-
$db_ref = null;
214-
if ($this->Testing) {
215-
$db_ref = &self::$testdb;
216-
if ($db_ref !== null) {
217-
return $db_ref;
218-
}
219-
//self::$testdb = Mock::generate("Database");
220-
self::$testdb = new MockDatabase();
221-
} else {
222-
$db_ref = &self::$db;
223-
if ($db_ref !== null) {
224-
return $db_ref;
225-
}
226-
self::$db = Database::singleton();
161+
$db_ref = &self::$db;
162+
if ($db_ref !== null) {
163+
return $db_ref;
227164
}
165+
228166
$config = $this->config();
229167
$dbc = $config->getSetting('database');
230168
// This check was added to satisfy phan, our static analysis tool.
@@ -235,13 +173,16 @@ class NDB_Factory
235173
'Could not configure database for LORIS'
236174
);
237175
}
238-
$db_ref->connect(
176+
177+
$db_ref = \Database::singleton(
239178
$dbc['database'],
240179
$dbc['username'],
241180
$dbc['password'],
242181
$dbc['host'],
243182
true
244183
);
184+
185+
self::$db = $db_ref;
245186
return $db_ref;
246187
}
247188

@@ -339,29 +280,16 @@ class NDB_Factory
339280
if (!empty(self::$_couchdb[$database])) {
340281
return self::$_couchdb[$database];
341282
}
342-
if ($this->Testing) {
343-
Mock::generatePartial(
344-
'CouchDB',
345-
'MockCouchDBWrap',
346-
/* mock out the functions that make HTTP requests */
347-
[
348-
'_getRelativeURL',
349-
'_postRelativeURL',
350-
'_postURL',
351-
'_getURL',
352-
]
353-
);
354-
self::$_couchdb[$database] = new MockCouchDBWrap();
355-
} else {
356-
self::$_couchdb[$database] = CouchDB::getInstance(
357-
$database,
358-
$host,
359-
$port,
360-
$user,
361-
$password
362-
);
363-
}
364-
return self::$_couchdb[$database];
283+
284+
$couch = CouchDB::getInstance(
285+
$database,
286+
$host,
287+
$port,
288+
$user,
289+
$password
290+
);
291+
self::$_couchdb[$database] = $couch;
292+
return $couch;
365293
}
366294

367295
/**
@@ -419,15 +347,7 @@ class NDB_Factory
419347
*/
420348
public function project(string $projectName): \Project
421349
{
422-
$project = null;
423-
424-
if ($this->Testing) {
425-
$project = new MockProject($projectName);
426-
} else {
427-
$project = \Project::singleton($projectName);
428-
}
429-
430-
return $project;
350+
return \Project::singleton($projectName);
431351
}
432352

433353
/**
@@ -443,11 +363,7 @@ class NDB_Factory
443363
if (isset(self::$_candidates[$key])) {
444364
return self::$_candidates[$key];
445365
}
446-
if ($this->Testing) {
447-
self::$_candidates[$key] = new MockCandidate($CandID);
448-
} else {
449-
self::$_candidates[$key] = Candidate::singleton($CandID);
450-
}
366+
self::$_candidates[$key] = Candidate::singleton($CandID);
451367
return self::$_candidates[$key];
452368

453369
}
@@ -465,15 +381,9 @@ class NDB_Factory
465381
if (isset(self::$_timepoints[(string) $sessionID])) {
466382
return self::$_timepoints[(string) $sessionID];
467383
}
468-
if ($this->Testing) {
469-
self::$_timepoints[(string) $sessionID] = new MockTimepoint(
470-
$sessionID
471-
);
472-
} else {
473-
self::$_timepoints[(string) $sessionID] = \TimePoint::singleton(
474-
$sessionID
475-
);
476-
}
384+
self::$_timepoints[(string) $sessionID] = \TimePoint::singleton(
385+
$sessionID
386+
);
477387
return self::$_timepoints[(string) $sessionID];
478388
}
479389
}

‎php/libraries/Utility.class.inc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -851,8 +851,10 @@ class Utility
851851
"Input must be a valid date/time string: $e"
852852
);
853853
}
854+
855+
$factory = \NDB_Factory::singleton();
854856
return $dt->format(
855-
\NDB_Config::singleton()->getSetting('dateDisplayFormat')
857+
$factory->config()->getSetting('dateDisplayFormat')
856858
?? DateTime::ATOM
857859
);
858860
}

‎test/integrationtests/LorisIntegrationTest.class.inc‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,22 @@ abstract class LorisIntegrationTest extends TestCase
6464
// Set up database wrapper and config
6565
$this->factory = NDB_Factory::singleton();
6666
$this->factory->reset();
67-
$this->factory->setTesting(false);
6867

6968
$this->config = $this->factory->Config(CONFIG_XML);
7069

7170
$database = $this->config->getSetting('database');
7271

73-
$this->DB = Database::singleton(
72+
$this->DB = Database::singleton(
7473
$database['database'],
7574
$database['username'],
7675
$database['password'],
7776
$database['host'],
7877
1
7978
);
79+
80+
$this->factory->setDatabase($this->DB);
81+
$this->factory->setConfig($this->config);
82+
8083
$this->url = getenv('DOCKER_WEB_SERVER');
8184
$password = new \Password($this->validPassword);
8285

‎test/unittests/CandidateTest.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ protected function setUp(): void
148148
$this->_configMock = $this->getMockBuilder('NDB_Config')->getMock();
149149
$this->_dbMock = $this->getMockBuilder('Database')->getMock();
150150
$this->_factory = NDB_Factory::singleton();
151+
151152
$this->_factory->setConfig($this->_configMock);
152153
$this->_factory->setDatabase($this->_dbMock);
153154

@@ -1311,7 +1312,6 @@ private function _setUpMockDB()
13111312
{
13121313
$this->_factoryForDB = NDB_Factory::singleton();
13131314
$this->_factoryForDB->reset();
1314-
$this->_factoryForDB->setTesting(false);
13151315
$this->_config = $this->_factoryForDB->Config(CONFIG_XML);
13161316
$database = $this->_config->getSetting('database');
13171317
$this->_DB = Database::singleton(
@@ -1321,5 +1321,8 @@ private function _setUpMockDB()
13211321
$database['host'],
13221322
1
13231323
);
1324+
1325+
$this->_factoryForDB->setDatabase($this->_DB);
1326+
$this->_factoryForDB->setConfig($this->_config);
13241327
}
13251328
}

‎test/unittests/Database_Test.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,6 @@ protected function setUp(): void
8686
{
8787
$this->factory = NDB_Factory::singleton();
8888
$this->factory->reset();
89-
$this->factory->setTesting(false);
9089
$this->config = $this->factory->Config(CONFIG_XML);
9190
$database = $this->config->getSetting('database');
9291
$this->DB = Database::singleton(
@@ -96,6 +95,9 @@ protected function setUp(): void
9695
$database['host'],
9796
1
9897
);
98+
99+
$this->factory->setDatabase($this->DB);
100+
$this->factory->setConfig($this->config);
99101
}
100102

101103
/**

‎test/unittests/NDB_BVL_Instrument_LINST_ToJSON_Test.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ function setUp(): void
5050
];
5151

5252
$factory = \NDB_Factory::singleton();
53-
$factory->setTesting(true);
5453

5554
$mockdb = $this->getMockBuilder("\Database")->getMock();
5655
$mockconfig = $this->getMockBuilder("\NDB_Config")->getMock();

‎test/unittests/NDB_BVL_Instrument_Test.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,12 @@ function setUp(): void
6565
];
6666

6767
$this->_factory = \NDB_Factory::singleton();
68-
$this->_factory->setTesting(true);
6968

7069
$this->_mockDB = $this->getMockBuilder("\Database")->getMock();
7170
$this->_mockConfig = $this->getMockBuilder("\NDB_Config")->getMock();
7271

73-
\NDB_Factory::$db = $this->_mockDB;
74-
\NDB_Factory::$testdb = $this->_mockDB;
75-
\NDB_Factory::$config = $this->_mockConfig;
72+
$this->_factory->setDatabase($this->_mockDB);
73+
$this->_factory->setConfig($this->_mockConfig);
7674

7775
$this->quickForm = new \LorisForm();
7876

@@ -1955,7 +1953,7 @@ private function _setUpMockDB()
19551953
{
19561954
$this->_factoryForDB = \NDB_Factory::singleton();
19571955
$this->_factoryForDB->reset();
1958-
$this->_factoryForDB->setTesting(false);
1956+
19591957
$this->_config = $this->_factoryForDB->Config(CONFIG_XML);
19601958
$database = $this->_config->getSetting('database');
19611959
$this->_DB = \Database::singleton(
@@ -1965,6 +1963,9 @@ private function _setUpMockDB()
19651963
$database['host'],
19661964
1
19671965
);
1966+
1967+
$this->_factoryForDB->setDatabase($this->_DB);
1968+
$this->_factoryForDB->setConfig($this->_config);
19681969
}
19691970
}
19701971

0 commit comments

Comments
 (0)