From 330ecf402e4c1c41210d2f178716b2d3a8a9f390 Mon Sep 17 00:00:00 2001 From: Dave MacFarlane Date: Mon, 7 Nov 2022 11:55:09 -0500 Subject: [PATCH] PHPCS --- .../php/instrumentqueryengine.class.inc | 301 ++++++++++++------ 1 file changed, 212 insertions(+), 89 deletions(-) diff --git a/modules/instruments/php/instrumentqueryengine.class.inc b/modules/instruments/php/instrumentqueryengine.class.inc index 3d66dd49576..03b193cb6ee 100644 --- a/modules/instruments/php/instrumentqueryengine.class.inc +++ b/modules/instruments/php/instrumentqueryengine.class.inc @@ -31,16 +31,20 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine { protected $loris; - public function __construct(\LORIS\LorisInstance $loris) { + /** + * Constructor + * + * @param \LORIS\LorisInstance $loris The LORIS instance + */ + public function __construct(\LORIS\LorisInstance $loris) + { $this->loris = $loris; } + /** * Return the data dictionary for all instruments installed on a LORIS * instance. * - * @param \LORIS\LorisInstance $loris The loris instance whose dictionary - * should be retrieved - * * @return \LORIS\Data\Dictionary\Category[] */ public function getDataDictionary() : iterable @@ -52,7 +56,12 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine $dict = []; foreach ($rows as $testname) { try { - $inst = \NDB_BVL_Instrument::factory($this->loris, $testname, "", ""); + $inst = \NDB_BVL_Instrument::factory( + $this->loris, + $testname, + "", + "", + ); $cat = new \LORIS\Data\Dictionary\Category( $testname, $inst->getFullName() @@ -66,32 +75,49 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine return $dict; } - public function getCandidateMatches(\LORIS\Data\Query\QueryTerm $term, ?array $visitlist=null) : iterable { - // This is stupid, but the parameter_type_override table uses '_' as a delimiter - // between instrument and fieldname, despite the fact that either one may itself - // have a _ in the name. This can't be easily changed without losing all - // existing overrides from the old datadict module. + /** + * {@inheritDoc} + * + * @param \LORIS\Data\Query\QueryTerm $term The criteria being matched + * @param ?array $visitlist The list of visits that must + * match for session scoped + * variables + * + * @return iterable + */ + public function getCandidateMatches( + \LORIS\Data\Query\QueryTerm $term, + ?array $visitlist=null + ) : iterable { + // This is stupid, but the parameter_type_override table uses '_' as + // a delimiter between instrument and fieldname, despite the fact that + // either one may itself have a _ in the name. This can't be easily + // changed without losing all existing overrides from the old datadict + // module. // - // We walk the existing test names ordered by length to look for the longest prefix - // match to find out what the instrument for the fieldname is. - $DB = $this->loris->getDatabaseConnection(); - $rows = $DB->pselectCol("SELECT Test_name FROM test_names ORDER BY Length(Test_name) DESC", []); + // We walk the existing test names ordered by length to look for the + // longest prefix match to find out what the instrument for the fieldname is. + $DB = $this->loris->getDatabaseConnection(); + $rows = $DB->pselectCol( + "SELECT Test_name FROM test_names ORDER BY Length(Test_name) DESC", + [] + ); - $testname = null; + $testname = null; $fieldname = null; - $fullname = $term->getDictionaryItem()->getName(); - foreach($rows as $testcandidate) { + $fullname = $term->getDictionaryItem()->getName(); + foreach ($rows as $testcandidate) { if (strpos($fullname, $testcandidate) === 0) { - $testname = $testcandidate; + $testname = $testcandidate; $fieldname = substr($fullname, strlen($testname)+1); break; } } - if($testname === null) { + if ($testname === null) { throw new \DomainException("Field for unknown instrument"); } - $query = "SELECT c.CandID, f.CommentID + $query = "SELECT c.CandID, f.CommentID FROM flag f JOIN session s ON (s.ID=f.SessionID AND s.Active='Y') JOIN candidate c ON (s.CandID=c.CandID AND c.Active='Y') @@ -108,27 +134,51 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine } $query .= ')'; } - $data = $DB->pselect($query, $queryparams); - $inst = \NDB_BVL_Instrument::factory($testname); - $values = $inst->bulkLoadInstanceData(array_map(function($row) { - return $row['CommentID']; - }, $data)); + $data = $DB->pselect($query, $queryparams); + $inst = \NDB_BVL_Instrument::factory($testname); + $values = $inst->bulkLoadInstanceData( + array_map( + function ($row) { + return $row['CommentID']; + }, + $data + ) + ); $map = []; - foreach($data as $row) { + foreach ($data as $row) { $map[$row['CommentID']] = new CandID($row['CandID']); } - return $this->filtered($values, $map, $fieldname, $term->getCriteria()); + return $this->_filtered($values, $map, $fieldname, $term->getCriteria()); } - private function filtered($values, $candidmap, $fieldname, $criteria) : \Traversable { - foreach($values as $inst) { + /** + * Filter out candidates which do not match a criteria + * + * @param array $values The values from the + * instrument + * @param array $candidmap CandID map from + * commentID=>candID + * commentID=>candID + * @param string $fieldname The field to compare + * @param \LORIS\Data\Query\Criteria $criteria The criteria which the field + * must match. + * + * @return \Traversable CandIDs that matched criteria + */ + private function _filtered( + $values, + $candidmap, + $fieldname, + $criteria + ) : \Traversable { + foreach ($values as $inst) { $value = $inst->getFieldValue($fieldname); - switch(get_class($criteria)) { + switch (get_class($criteria)) { case \LORIS\Data\Query\Criteria\In::class: - foreach($criteria->getValue() as $valuecandidate) { - if($value == $valuecandidate) { + foreach ($criteria->getValue() as $valuecandidate) { + if ($value == $valuecandidate) { yield $candidmap[$inst->getCommentID()]; } } @@ -182,15 +232,16 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine break; case \LORIS\Data\Query\Criteria\EndsWith::class: $critval = $criteria->getValue(); - if ($value !== null && - strrpos($value, $critval) - === strlen($value)-strlen($critval) - ) { + $matches = strrpos($value, $critval) + === strlen($value)-strlen($critval); + if ($value !== null && $matches) { yield $candidmap[$inst->getCommentID()]; } break; case \LORIS\Data\Query\Criteria\Substring::class: - if ($value !== null && strpos($value, $criteria->getValue()) !== false) { + if ($value !== null + && strpos($value, $criteria->getValue()) !== false + ) { yield $candidmap[$inst->getCommentID()]; } break; @@ -201,41 +252,67 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine } private $visitcache = []; - public function getVisitList(\LORIS\Data\Dictionary\Category $inst, \LORIS\Data\Dictionary\DictionaryItem $item) : iterable - { - if($item->getScope()->__toString() !== 'session') { + + /** + * Get list of visits that an instrument is valid at + * + * @param \LORIS\Data\Dictionary\Category $inst The instrument + * category + * @param \LORIS\Data\Dictionary\DictionaryItem $item The field + * + * @return string[] + */ + public function getVisitList( + \LORIS\Data\Dictionary\Category $inst, + \LORIS\Data\Dictionary\DictionaryItem $item + ) : iterable { + if ($item->getScope()->__toString() !== 'session') { return null; } // An instrument's fields all have the same visit list, so cache // the results to ensure it's only queried once. - if(isset($this->visitcache[$inst->getName()])) { + if (isset($this->visitcache[$inst->getName()])) { return $this->visitcache[$inst->getName()]; } - $DB = \NDB_Factory::singleton()->database(); - $visits = $DB->pselectCol("SELECT DISTINCT s.Visit_Label - FROM flag f - JOIN session s ON (f.SessionID=s.ID) - JOIN candidate c ON (c.CandID=s.CandID) - WHERE s.Active='Y' AND c.Active='Y' and f.Test_name=:tn - ORDER BY s.Visit_label", + $DB = \NDB_Factory::singleton()->database(); + $visits = $DB->pselectCol( + "SELECT DISTINCT s.Visit_Label + FROM flag f + JOIN session s ON (f.SessionID=s.ID) + JOIN candidate c ON (c.CandID=s.CandID) + WHERE s.Active='Y' AND c.Active='Y' and f.Test_name=:tn + ORDER BY s.Visit_label", ['tn' => $inst->getName()] ); $this->visitcache[$inst->getName()] = $visits; - + return $visits; } - public function getCandidateData(array $items, iterable $candidates, ?array $visitlist) : iterable { - // We need to know what category the item is in to get the instrument, so get the - // full data dictionary and go through each to see if it's the same as $item + /** + * {inheritDoc} + * + * @param array $items DictionaryItems for which to get data + * @param iterable $candidates Candidates whose data we want + * @param ?array $visitlist List of visits that we want data for + * + * @return iterable CandID => items data + */ + public function getCandidateData( + array $items, + iterable $candidates, + ?array $visitlist + ) : iterable { + // We need to know what category the item is in to get the instrument, + // so get the full data dictionary and go through each to see if + // it's the same as $item $field2instMap = []; - $fullDictionary = $this->getDataDictionary(); - foreach($fullDictionary as $category) { + foreach ($fullDictionary as $category) { $instrument = $category->getName(); foreach ($category->getItems() as $dict) { $field2instMap[$dict->getName()] = $instrument; @@ -243,10 +320,10 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine } $instruments = []; - foreach($items as $dict) { + foreach ($items as $dict) { $instr = $field2instMap[$dict->getName()]; - if(!in_array($instr, $instruments, true)) { + if (!in_array($instr, $instruments, true)) { $instruments[] = $instr; } @@ -257,22 +334,29 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine // clause. Directly using "c.CandID IN (candid1, candid2, candid3, etc)" is // too slow. $DB->run("DROP TEMPORARY TABLE IF EXISTS querycandidates"); - $DB->run("CREATE TEMPORARY TABLE querycandidates ( - CandID int(6) - );"); - $insertstmt = "INSERT INTO querycandidates VALUES (" . join('),(', $candidates) . ')'; + $DB->run( + "CREATE TEMPORARY TABLE querycandidates ( + CandID int(6) + );" + ); + $insertstmt = "INSERT INTO querycandidates VALUES (" + . join('),(', $candidates) + . ')'; + $q = $DB->prepare($insertstmt); $q->execute([]); - $rows = $DB->pselect("SELECT c.CandID, CommentID FROM flag f - JOIN session s ON (f.SessionID=s.ID) - JOIN candidate c ON (s.CandID=c.CandID) - WHERE f.CommentID NOT LIKE 'DDE%' - AND c.CandID IN (SELECT CandID FROM querycandidates) - AND f.Test_name IN ('" . join("', '", $instruments). "') - AND c.Active='Y' AND s.Active='Y' - ORDER BY c.CandID", - []); + $rows = $DB->pselect( + "SELECT c.CandID, CommentID FROM flag f + JOIN session s ON (f.SessionID=s.ID) + JOIN candidate c ON (s.CandID=c.CandID) + WHERE f.CommentID NOT LIKE 'DDE%' + AND c.CandID IN (SELECT CandID FROM querycandidates) + AND f.Test_name IN ('" . join("', '", $instruments). "') + AND c.Active='Y' AND s.Active='Y' + ORDER BY c.CandID", + [], + ); $data = []; foreach ($candidates as $candidate) { @@ -285,30 +369,51 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine } $instrumentIterators = []; - foreach($instruments as $instrument) { - $mStartMemory = memory_get_usage(); - $now = microtime(true); - $inst = \NDB_BVL_Instrument::factory($this->loris, $instrument); - $values = $inst->bulkLoadInstanceData(array_map(function($row) { + foreach ($instruments as $instrument) { + $inst = \NDB_BVL_Instrument::factory($this->loris, $instrument); + $values = $inst->bulkLoadInstanceData( + array_map( + function ($row) { return $row['CommentID']; - }, $rows)); - $instrumentIterators[$instrument] = $this->dataToIterator($values, $commentID2CandID, $items, $field2instMap); + }, + $rows + ), + ); + + $instrumentIterators[$instrument] = $this->_dataToIterator( + $values, + $commentID2CandID, + $items, + $field2instMap + ); } - return $this->mergeIterators($candidates, $instrumentIterators); + return $this->_mergeIterators($candidates, $instrumentIterators); } - private function mergeIterators ($candidates, $iterators) { - foreach($candidates as $candID) { + /** + * Merge the iterators for each instrument into a single iterator for the + * candidate. + * + * @param string[] $candidates The list of CandIDs that were expected to + be returned + * @param iterable[] $iterators An iterator for each instrument, may or + * may not have every CandID + * + * @return iterable + */ + private function _mergeIterators($candidates, $iterators) + { + foreach ($candidates as $candID) { // Go through each module, if it has data for this candidate // put it in the appropriate columns. $candidateData = []; - $candIDStr = "$candID"; - foreach($iterators as $instrumentName => $instrData ) { - if(!$instrData->valid()) { + $candIDStr = "$candID"; + foreach ($iterators as $instrumentName => $instrData ) { + if (!$instrData->valid()) { continue; } $instCandidate = $instrData->key(); - // The candidate data must have been sorted by + // The candidate data must have been sorted by // CandID for our logic to work. // Coerce to string for the comparison so that <= // works. (We can't do a <= comparison on a CandID @@ -318,7 +423,6 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine // If the module has data for this candID, populate it, // if not, don't advance the iterator. while ($instCandidate == $candIDStr && $instrData->valid()) { - // while ("$instCandidate" == "$candID" && $instrData->valid()) { // Go through each field and put the data in the right // index if applicable. $data = $instrData->current(); @@ -336,18 +440,37 @@ class InstrumentQueryEngine implements \LORIS\Data\Query\QueryEngine } } - private function dataToIterator($values, $commentID2CandID, $items, $field2instMap) { + /** + * Take the data loaded by bulkLoadInstrumentData and convert it to an + * iterator of the format expected by a QueryEngine + * + * @param array $values Loaded instruments with data + * @param array $commentID2CandID A Mapping from CommentID to CandID + * @param array $items The QueryEngine items selected + * @param array $field2instMap A Map of fields to which instrument + * the field is from for each item + * in $items + * + * @return iterable + */ + private function _dataToIterator( + $values, + $commentID2CandID, + $items, + $field2instMap, + ) { foreach ($values as $loadedInstrument) { $candData = []; - $iCandID = $commentID2CandID[$loadedInstrument->getCommentID()]; + $iCandID = $commentID2CandID[$loadedInstrument->getCommentID()]; - foreach($items as $idx => $dict) { + foreach ($items as $idx => $dict) { $fieldinst = $field2instMap[$dict->getName()]; if ($fieldinst == $loadedInstrument->testName) { if (!isset($candData[$dict->getName()])) { $candData[$dict->getName()] = []; - } - $candData[$dict->getName()][] = $loadedInstrument->getDictionaryValue($dict); + } + $candData[$dict->getName()][] + = $loadedInstrument->getDictionaryValue($dict); } } yield "$iCandID" => $candData;