Skip to content

Commit

Permalink
[instrument_manager] Use JSON data for uploaded linst instruments (ac…
Browse files Browse the repository at this point in the history
…es#9324)

This updates the instrument_manager to create a .meta file for uploaded
LINST instruments to use JSON for saving data, rather than running the
generate_tables_and_test_names script. This is more robust, doesn't
require exec'ing mysql, doesn't require create table permission for the
upload, and better aligns with LORIS best practices to use JSON instead
of table data by default.
  • Loading branch information
driusan authored and maximemulder committed Sep 24, 2024
1 parent bb8ce19 commit 8f2b503
Showing 1 changed file with 23 additions and 97 deletions.
120 changes: 23 additions & 97 deletions modules/instrument_manager/php/instrument_manager.class.inc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class Instrument_Manager extends \DataFrameworkMenu
*/
protected function handlePOST(ServerRequestInterface $request): ResponseInterface
{
$loris = $request->getAttribute('loris');
// Ensure the user is allowed to upload.
if (! $request->getAttribute('user')->hasPermission(
'instrument_manager_write'
Expand All @@ -94,7 +93,6 @@ class Instrument_Manager extends \DataFrameworkMenu
$filename = $uploaded_file->getClientFilename();
$instrument = pathinfo($filename)['filename'];
$targetdir = new \SplFileInfo($this->_path . 'project/instruments/');
$fullpath = $targetdir->getPathname() . '/' . $filename;

if ($this->instrumentExists($instrument)) {
// An instrument with this name already exists in the test_names
Expand Down Expand Up @@ -128,37 +126,33 @@ class Instrument_Manager extends \DataFrameworkMenu
return $response;
}

// Scripts in tools/ often make relative imports, so we must change
// our effective directory in order to use them.
chdir($this->_path . "/tools");
// Use tools/ script to generate an SQL patch file based on the
// structure of the uploaded .linst file.
exec(
'php generate_tables_sql_and_testNames.php < '
. escapeshellarg($fullpath)
);

if (!$this->isAdminUserConfigured()) {
// If no adminUser is configured, automatic installation is not
// possible, so this is the last step.
return new \LORIS\Http\Response\JSON\OK(
['success' => self::UPLOAD_NO_INSTALL]
);
}

// Install the instrument by directly sourcing the SQL file
// generated by `generate_tables_sql_and_testNames.php` using bash.
// If installation is successful, `exec` will complete
// silently. Otherwise, it will return the exit code and error
// messsage from MySQL. This will be stored in $result and
// logged via LorisException.
try {
$table_name = \NDB_BVL_Instrument::factory(

$inst = \NDB_BVL_Instrument::factory(
$this->loris,
$instrument,
'',
'',
)->table;
);

$DB = $this->loris->getDatabaseConnection();
$DB->insert(
"test_names",
[
'test_name' => $instrument,
'Full_name' => $inst->getFullName(),
'Sub_group' => 1,
]
);
file_put_contents(
\Utility::pathJoin(
$targetdir->getPath(),
$targetdir->getFilename(),
$instrument . ".meta"
),
"jsondata{@}true\n"
);

} catch (\NotFound $e) {
return (new \LORIS\Http\Response\JSON\NotFound(
$e->getMessage()
Expand All @@ -168,34 +162,6 @@ class Instrument_Manager extends \DataFrameworkMenu
$e->getMessage()
);
}

$db_config = $loris->getConfiguration()->getSetting('database');
exec(
"mysql".
" -h" . escapeshellarg($db_config['host']).
" -u" . escapeshellarg($db_config['adminUser']).
" -p" . escapeshellarg($db_config['adminPassword']).
" " . escapeshellarg($db_config['database']).
" < " . $this->_path . "project/tables_sql/".
escapeshellarg($table_name . '.sql'),
$output, // $output and $status are created automatically
$status // by `exec` and so need not be declared above.
);
// An exit code of 0 is a success and 1 means failure
if ($status) {
$this->logger->error(
"The installation of $instrument.sql failed. "
. "Either: the instrument table exists (but is not in the "
. "test_names table), or "
. "LORIS could not connect to the database using the "
. "credentials supplied in the config file. "
. "Exec output: `" . implode("\n", $output) . "`"
);
return new \LORIS\Http\Response\JSON\InternalServerError(
self::UPLOAD_INSTALL_FAILED
);
}

return new \LORIS\Http\Response\JSON\Created(["ok"=>"ok"]);
}
/**
Expand Down Expand Up @@ -232,7 +198,6 @@ class Instrument_Manager extends \DataFrameworkMenu
return [
'allPermissionCodes' => $perms,
'writable' => $this->canWriteFiles(),
'caninstall' => $this->isAdminUserConfigured()
];

}
Expand All @@ -255,49 +220,10 @@ class Instrument_Manager extends \DataFrameworkMenu
{
$this->_path = $this->loris->getConfiguration()->getSetting('base');
$instrument_dir = $this->_path . 'project/instruments';
$table_dir = $this->_path . 'project/tables_sql';

return is_writable($instrument_dir) && is_writable($table_dir);
return is_writable($instrument_dir);
}
/**
* Return whether the adminUser is properly configured, ie. credentials are
* set and are valid.
* The adminUser is a MySQL user with CREATE table permissions.
* `empty` is used instead of `isset` as blank values in the config file
* are still considered set.
*
* @return bool True if a adminUser is configured properly. False if not.
*/
protected function isAdminUserConfigured() : bool
{
$db = $this->loris->getDatabaseConnection();
$db_config = $this->loris->getConfiguration()->getSetting('database');

$credentials_set = !empty($db_config['adminUser'])
&& !empty($db_config['adminPassword']);
if (!$credentials_set) {
return false;
}
// Check if supplied credentials are valid by making a DB connection.
// If the credentials are invalid, an error message will be logged to
// the backend.
try {
$dbname = $db_config['database'];
putenv("LORIS_{$dbname}_USERNAME=" . $db_config['adminUser']);
putenv("LORIS_{$dbname}_PASSWORD=" . $db_config['adminPassword']);
putenv("LORIS_{$dbname}_HOST=" . $db_config['host']);
$connected = $db->connect(
$dbname,
false
);
putenv("LORIS_{$dbname}_USERNAME=");
putenv("LORIS_{$dbname}_PASSWORD=");
putenv("LORIS_{$dbname}_HOST=");
} catch (\DatabaseException $e) {
$connected = false;
}
return $connected;
}
/**
* Checks the test_names table for the existence of the instrument
*
Expand Down

0 comments on commit 8f2b503

Please sign in to comment.