Skip to content

Commit

Permalink
[Instrument] Make instrument data non-static (#6868)
Browse files Browse the repository at this point in the history 
NDB_BVL_Instrument has an.. unusual.. way of dealing with instrument
data. Rather than having a class method for getting the data, there's
a static method which takes an instrument as a parameter. The result
is that data can not be cached in the object and needs to be continuously
re-loaded from the database every time if it's needed multiple times in
the same request. (For instance, calling getFieldValue for different fields
in a script.)

This adds a normal getter and caches the results in the object, deprecating
the old static loadInstanceData.
  • Loading branch information
@driusan
driusan authored Nov 20, 2020
1 parent 4a8d4f2 commit 4621058
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 26 deletions.
5 changes: 1 addition & 4 deletions modules/api/php/views/visit/flags.class.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ class Flags
public function toArray(): array
{
$instrumentname = $this->_instrument->testName;

$instrumentdata = \NDB_BVL_Instrument::loadInstanceData(
$this->_instrument
);
$instrumentdata = $this->_instrument->getInstanceData();

$isDDE = strpos($instrumentdata['CommentID'], 'DDE_') === 0;

Expand Down
5 changes: 1 addition & 4 deletions modules/api/php/views/visit/instrument.class.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,7 @@ class Instrument
public function toArray(): array
{
$instrumentname = $this->_instrument->testName;

$instrumentdata = \NDB_BVL_Instrument::loadInstanceData(
$this->_instrument
);
$instrumentdata = $this->_instrument->getInstanceData();

$isDDE = strpos($instrumentdata['CommentID'], 'DDE_') === 0;

Expand Down
63 changes: 54 additions & 9 deletions php/libraries/NDB_BVL_Instrument.class.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ abstract class NDB_BVL_Instrument extends NDB_Page
*/
public $commentID;

/**
* The data associated with the commentID for this instrument
*
* @var ?array
*/
protected $instanceData = null;

/**
* Database table containing data referenced by $this->commentID
*
Expand Down Expand Up @@ -464,7 +471,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
$this->addGroup($buttons, null, null, " ");
}

$defaults = $this->loadInstanceData($this);
$defaults = $this->getInstanceData();

// set the defaults (call private method _setDefaultsArray
// which could be overridden if necessary)
Expand Down Expand Up @@ -822,7 +829,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
["Data" => json_encode($newData)],
['CommentID' => $this->getCommentID()]
);

$this->instanceData = $newData;
}


Expand Down Expand Up @@ -1437,7 +1444,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
*/
function getFieldValue(string $field)
{
$allValues = self::loadInstanceData($this);
$allValues = $this->getInstanceData();

if (!array_key_exists($field, $allValues)) {
throw new \OutOfBoundsException("Invalid field for instrument");
Expand Down Expand Up @@ -1507,7 +1514,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
if (empty($this->_requiredElements)) {
return 100;
}
$allData = $this->loadInstanceData($this);
$allData = $this->getInstanceData();
$unanswered = 0;
$totalElements = count($this->_requiredElements);
foreach ($this->_requiredElements as $field) {
Expand Down Expand Up @@ -1543,7 +1550,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
return 'Complete';
}

$allData = $this->loadInstanceData($this);
$allData = $this->getInstanceData();
foreach ($this->_requiredElements as $field) {
// consider status field if not_answered chosen as an answer
$statusField = $field . '_status';
Expand All @@ -1567,7 +1574,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
*/
function getDataEntryCompletionStatus(): string
{
$data = NDB_BVL_Instrument::loadInstanceData($this);
$data = $this->getInstanceData();
return $data["Data_entry_completion_status"];
}

Expand Down Expand Up @@ -2053,7 +2060,7 @@ abstract class NDB_BVL_Instrument extends NDB_Page
*/
function _nullScores(array $scoreCols): void
{
$data = $this->loadInstanceData($this);
$data = $this->getInstanceData();

// set the scoring cols to NULL
foreach ($scoreCols as $key => $val) {
Expand Down Expand Up @@ -2086,10 +2093,10 @@ abstract class NDB_BVL_Instrument extends NDB_Page
function diff(NDB_BVL_Instrument $otherInstrument): array
{
// Load this instance data
$thisData = NDB_BVL_Instrument::loadInstanceData($this);
$thisData = $this->getInstanceData();

// Load other instance data
$otherData = NDB_BVL_Instrument::loadInstanceData($otherInstrument);
$otherData = $otherInstrument->getInstanceData();

// Create the return object data structure
$diff = [];
Expand Down Expand Up @@ -2117,6 +2124,38 @@ abstract class NDB_BVL_Instrument extends NDB_Page
return $diff;
}

/**
* Gets the data from an instrument out of the database and returns it
* as an array.
*
* @return array containing the data for each field in this instrument
*/
public function getInstanceData(): array
{
if ($this->instanceData !== null) {
return $this->instanceData;
}

$db = \Database::singleton();

if ($this->jsonData) {
$jsondata = $db->pselectOne(
"SELECT Data FROM flag WHERE CommentID=:CID",
['CID' => $this->getCommentID()]
);

$this->instanceData = json_decode($jsondata, true) ?? [];
} else {
$defaults = $db->pselectRow(
"SELECT * FROM $this->table WHERE CommentID=:CID",
['CID' => $this->getCommentID()]
);

$this->instanceData = $defaults ?? [];
}
return $this->instanceData;
}

/**
* Gets the data from an instrument out of the database and returns it
* as an array.
Expand All @@ -2125,9 +2164,15 @@ abstract class NDB_BVL_Instrument extends NDB_Page
* to be retrieved.
*
* @return array containing the data for each field in this instrument
*
* @deprecated use $instrumentInstance->getInstanceData() instead
*/
static function loadInstanceData(NDB_BVL_Instrument $instrumentInstance): array
{
error_log(
'Warning: loadInstanceData($inst) is deprecated and will be removed'
. ' in a future version of LORIS. Use $inst->getInstanceData() instead.'
);
$db = \Database::singleton();

if ($instrumentInstance->jsonData) {
Expand Down
2 changes: 1 addition & 1 deletion raisinbread/instruments/NDB_BVL_Instrument_aosi.class.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,7 @@ class NDB_BVL_Instrument_aosi extends NDB_BVL_Instrument
// null scores
$this->_nullScores($this->scores);

$score_record = NDB_BVL_Instrument::loadInstanceData($this);
$score_record = $this->getInstanceData();
//Calculate first 6 item scores from their input grids

//Question 1
Expand Down
14 changes: 7 additions & 7 deletions test/unittests/NDB_BVL_Instrument_Test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1126,18 +1126,18 @@ function testGetPSCID()
}

/**
* Test that loadInstanceData returns data from the correct database
* Test that getInstanceData returns data from the correct database
*
* @covers NDB_BVL_Instrument::loadInstanceData
* @covers NDB_BVL_Instrument::getInstanceData
* @return void
*/
function testLoadInstanceData()
function testGetInstanceData()
{
$this->_setUpMockDB();
$this->_setTableData();
$this->_instrument->commentID = 'commentID1';
$this->_instrument->table = 'flag';
$defaults = \NDB_BVL_Instrument::loadInstanceData($this->_instrument);
$defaults = $this->_instrument->getInstanceData();
$defaults['Testdate'] = '2020-01-01 00:00:00';
$this->assertEquals(
$defaults,
Expand Down Expand Up @@ -1563,7 +1563,7 @@ function testSetDataEntryCompletionStatus()
$this->_instrument->commentID = 'commentID1';
$this->_instrument->table = 'medical_history';
$this->_instrument->_setDataEntryCompletionStatus('Complete');
$data = \NDB_BVL_Instrument::loadInstanceData($this->_instrument);
$data = $this->_instrument->getInstanceData();
$this->assertEquals('Complete', $data['Data_entry_completion_status']);
}

Expand Down Expand Up @@ -1598,7 +1598,7 @@ function testNullScores()
$this->_instrument->commentID = 'commentID1';
$this->_instrument->table = 'medical_history';
$this->_instrument->_nullScores(['Examiner' => 'Test Examiner1']);
$data = \NDB_BVL_Instrument::loadInstanceData($this->_instrument);
$data = $this->_instrument->getInstanceData();
$this->assertEquals(null, $data['Examiner']);
}

Expand Down Expand Up @@ -1675,7 +1675,7 @@ function testClearInstrument()
[]
);
$this->_instrument->clearInstrument();
$data = \NDB_BVL_Instrument::loadInstanceData($this->_instrument);
$data = $this->_instrument->getInstanceData();
$conflictsAfter = $this->_DB->pselect(
"SELECT * FROM conflicts_unresolved",
[]
Expand Down
2 changes: 1 addition & 1 deletion tools/single_use/instrument_double_escape_report.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@
}
foreach ($instrumentCIDs as $cid) {
$instrumentInstance = \NDB_BVL_Instrument::factory($instrumentName, $cid);
$instrumentCandData = \NDB_BVL_Instrument::loadInstanceData($instrumentInstance);
$instrumentCandData = $instrumentInstance->getInstanceData();

// instrument name and table name might differ
$tableName = $instrumentInstance->table;
Expand Down

0 comments on commit 4621058

Please sign in to comment.