Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[imaging browser] Fix download of JSON, Bval and Bvec files when they are on s3 #8354

Merged
merged 24 commits into from
Feb 13, 2023
Merged

[imaging browser] Fix download of JSON, Bval and Bvec files when they are on s3 #8354

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a586f1e
add download of BIDS JSON, NIfTI + Bval + Bvec to the API so it can b…
cmadjar Feb 2, 2023
4716737
use API links to download from view session imaging browser
cmadjar Feb 2, 2023
c40102c
fix phan
cmadjar Feb 2, 2023
4f3ea96
fix phan
cmadjar Feb 2, 2023
fdb6512
create one class to do them all
cmadjar Feb 2, 2023
8bf64cf
move switch to the constructor of the class
cmadjar Feb 2, 2023
46aa398
fix phpcbf
cmadjar Feb 2, 2023
e18586b
fix phpcbf
cmadjar Feb 2, 2023
998b34f
fix phpcbf
cmadjar Feb 2, 2023
d0eb91f
fix phpcbf
cmadjar Feb 2, 2023
03a3d42
fix phpcbf
cmadjar Feb 2, 2023
8c1a236
Xavier's feedback I missed
cmadjar Feb 2, 2023
3f86d70
revert last commit
cmadjar Feb 2, 2023
1cdce27
Xavier's feedback after a good night sleep
cmadjar Feb 3, 2023
614b334
remove all ->_check variables
cmadjar Feb 3, 2023
0de005e
remove unecessary extra comma
cmadjar Feb 3, 2023
c1e92b2
remove unecessary extra comma
cmadjar Feb 3, 2023
a76d31c
Xavier's feedback
cmadjar Feb 10, 2023
a24a5da
Make Phan happy
cmadjar Feb 10, 2023
f6e54ff
PSR
cmadjar Feb 13, 2023
4fe033a
Phan
cmadjar Feb 13, 2023
55c8dfd
Phan
cmadjar Feb 13, 2023
309f52b
Dave's comment
cmadjar Feb 13, 2023
27c14a5
Dave's comment
cmadjar Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion modules/api/php/endpoints/candidate/visit/image/format.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,11 @@ class Format extends Endpoint
$handler = new Format\Brainbrowser($this->_image);
break;
case 'thumbnail':
$handler = new Format\Thumbnail($this->_image);
case 'nifti':
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't thumbnails intended to be displayed inline?

(It looks like the code already had an attachment Content-Disposition, so I guess it doesn't matter..)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the thumbnail is mainly used to be displayed in the imaging browser but it was already available for download in the API and that is the file I used to base all the other classes so I kept it. @driusan does it answer your question?

case 'bval':
case 'bvec':
case 'bidsjson':
$handler = new Format\DownloadFile($this->_image, $format);
break;
default:
return new \LORIS\Http\Response\JSON\UnsupportedMediaType();
Expand Down
72 changes: 52 additions & 20 deletions ...te/visit/image/format/thumbnail.class.inc → ...visit/image/format/downloadfile.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use \LORIS\api\Endpoint;
* @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
* @link https://github.com/aces/Loris
*/
class Thumbnail extends Endpoint implements \LORIS\Middleware\ETagCalculator
class DownloadFile extends Endpoint implements \LORIS\Middleware\ETagCalculator
{

/**
Expand All @@ -32,15 +32,49 @@ class Thumbnail extends Endpoint implements \LORIS\Middleware\ETagCalculator
* @var \LORIS\Image
*/
private $_image;
xlecours marked this conversation as resolved.
Show resolved Hide resolved
private $_format;
private $_info;
private $_file_type;
private $_content_type;

/**
* Contructor
*
* @param \LORIS\Image $image The requested image
* @param \LORIS\Image $image The requested image
* @param string $format The format user wishes to download
*/
public function __construct(\LORIS\Image $image)
public function __construct(\LORIS\Image $image, string $format)
{
$this->_image = $image;
$this->_image = $image;
xlecours marked this conversation as resolved.
Show resolved Hide resolved
$this->_format = $format;

switch ($this->_format) {
case 'thumbnail':
$this->_info = $this->_image->getThumbnailFileInfo();
xlecours marked this conversation as resolved.
Show resolved Hide resolved
$this->_file_type = "Thumbnail";
$this->_content_type = 'image/jpeg';
break;
case 'nifti':
$this->_info = $this->_image->getNiiFileInfo();
xlecours marked this conversation as resolved.
Show resolved Hide resolved
$this->_file_type = "NIfTI";
$this->_content_type = 'application/octet-stream';
break;
case 'bval':
$this->_info = $this->_image->getBvalFileInfo();
xlecours marked this conversation as resolved.
Show resolved Hide resolved
$this->_file_type = "NIfTI BVAL";
$this->_content_type = 'application/text';
break;
case 'bvec':
$this->_info = $this->_image->getBvecFileInfo();
xlecours marked this conversation as resolved.
Show resolved Hide resolved
$this->_file_type = "NIfTI BVEC";
$this->_content_type = 'application/text';
break;
case 'bidsjson':
$this->_info = $this->_image->getBidsJsonFileInfo();
xlecours marked this conversation as resolved.
Show resolved Hide resolved
$this->_file_type = "BIDS JSON";
$this->_content_type = 'application/json';
break;
}
}

/**
Expand Down Expand Up @@ -100,23 +134,22 @@ class Thumbnail extends Endpoint implements \LORIS\Middleware\ETagCalculator
*/
private function _handleGET(): ResponseInterface
{
$info = $this->_image->getThumbnailFileInfo();

if (!$info->isFile()) {
error_log('Thumbnail not found');
if (!$this->_info->isFile()) {
error_log("$this->_file_type file not found");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be using the PSR logger, not error_log

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@driusan could you elaborate on how to do that? Sorry I am not very versed in the PSR realm apparently...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NDB_Page includes a $this->logger which implements PSR-3 (https://www.php-fig.org/psr/psr-3/) which honours the config settings from the LORIS config. I think this endpoint bypasses NDB_Page, so you might need to copy the initialization from the NDB_Page constructor and then use ie $this->logger->notice("something") (or the appropriate log level)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would involve changing src/Http/Endpoint to use Psr\Log\LoggerAwareTrait; and add somehting like this in process.

$loris = $request->getAttribute('loris');
$loglevel = $loris->getConfiguration()
    ->getLogSettings()
    ->getRequestLogLevel();

$this->logger = new \LORIS\Log\ErrorLogLogger($loglevel);

return new \LORIS\Http\Response\JSON\NotFound();
}

if (!$info->isReadable()) {
error_log('Thumbnail exists but is not readable by webserver');
if (!$this->_info->isReadable()) {
error_log(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

"$this->_file_type file exists but is not readable by webserver"
);
return new \LORIS\Http\Response\JSON\NotFound();
}

$filename = $info->getFilename();

$realpath = $info->getRealPath();
$filename = $this->_info->getFilename();
$realpath = $this->_info->getRealPath();
if ($realpath === false) {
$realpath = $info->getPath() . "/" . $info->getFilename();
$realpath = $this->_info->getPath() . "/" . $this->_info->getFilename();
}

$body = new \LORIS\Http\FileStream($realpath, 'r');
Expand All @@ -125,7 +158,7 @@ class Thumbnail extends Endpoint implements \LORIS\Middleware\ETagCalculator
$body,
200,
[
'Content-Type' => 'image/jpeg',
'Content-Type' => $this->_content_type,
'Content-Disposition' => 'attachment; filename=' . $filename,
]
);
Expand All @@ -140,16 +173,15 @@ class Thumbnail extends Endpoint implements \LORIS\Middleware\ETagCalculator
*/
public function ETag(ServerRequestInterface $request) : string
{
$info = $this->_image->getThumbnailFileInfo();

if (!$info->isFile() || !$info->isReadable()) {
if (!$this->_info->isFile() || !$this->_info->isReadable()) {
return '';
}

$signature = [
'filename' => $info->getFilename(),
'size' => $info->getSize(),
'mtime' => $info->getMTime(),
'filename' => $this->_info->getFilename(),
'size' => $this->_info->getSize(),
'mtime' => $this->_info->getMTime(),
];

return md5(json_encode($signature));
Expand Down
41 changes: 25 additions & 16 deletions modules/imaging_browser/jsx/ImagePanel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ DownloadButton.propTypes = {
FileName: PropTypes.string,
BaseURL: PropTypes.string,
Label: PropTypes.string,
URL: PropTypes.string,
};


Expand Down Expand Up @@ -826,22 +827,30 @@ class ImageDownloadButtons extends Component {
BaseURL={this.props.BaseURL}
Label="Download NRRD"
/>
<DownloadButton FileName={this.props.NiiFile}
BaseURL={this.props.BaseURL}
Label="Download NIfTI"
/>
<DownloadButton FileName={this.props.BvalFile}
BaseURL={this.props.BaseURL}
Label="Download BVAL"
/>
<DownloadButton FileName={this.props.BvecFile}
BaseURL={this.props.BaseURL}
Label="Download BVEC"
/>
<DownloadButton FileName={this.props.JsonFile}
BaseURL={this.props.BaseURL}
Label="Download BIDS JSON"
/>
{ this.props.NiiFile ?
<DownloadButton URL={this.props.APIFile + '/format/nifti'}
Label="Download NIfTI"
/> :
null
}
{this.props.BvalFile ?
<DownloadButton URL={this.props.APIFile + '/format/bval'}
Label="Download BVAL"
/> :
null
}
{this.props.BvecFile ?
<DownloadButton URL={this.props.APIFile + '/format/bvec'}
Label="Download BVEC"
/> :
null
}
{this.props.JsonFile ?
<DownloadButton URL={this.props.APIFile + '/format/bidsjson'}
Label="Download BIDS JSON"
/> :
null
}
<LongitudinalViewButton FileID={this.props.FileID}
BaseURL={this.props.BaseURL}
OtherTimepoints={this.props.OtherTimepoints}
Expand Down
1 change: 1 addition & 0 deletions modules/imaging_browser/php/viewsession.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ class ViewSession extends \NDB_Form
'OtherTimepoints' => $OtherTimepoints,
'CaveatViolationsResolvedID' => $caveatViolationsResolvedID,
];

$this->tpl_data['files'][] = $file;
}
}
Expand Down
48 changes: 44 additions & 4 deletions php/libraries/Image.class.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class Image
private \CenterID $_centerid;

private $_entitytype;

/**
* Constructor
*
Expand All @@ -58,7 +59,7 @@ class Image
s.CenterID as centerid,
c.Entity_type as entitytype
FROM
files f
files f
LEFT JOIN session s
ON (f.SessionID = s.ID)
LEFT JOIN candidate c
Expand All @@ -80,7 +81,6 @@ class Image
$this->_filetype = $dbrow['filetype'];
$this->_centerid = new \CenterID($dbrow['centerid']);
$this->_entitytype = $dbrow['entitytype'];

}
}

Expand All @@ -100,7 +100,7 @@ class Image
SELECT
Value
FROM
parameter_file pf
parameter_file pf
JOIN parameter_type pt
USING (ParameterTypeID)
JOIN files f
Expand Down Expand Up @@ -132,7 +132,7 @@ class Image
pt.Name as name,
pf.Value as value
FROM
parameter_file pf
parameter_file pf
JOIN parameter_type pt
USING (ParameterTypeID)
JOIN files f
Expand Down Expand Up @@ -253,6 +253,46 @@ class Image
return $this->_getFullPath($this->_filelocation);
}

/**
* Return a SPLFileInfo object based on this images's properties.
*
* @return \SplFileInfo
*/
public function getNiiFileInfo(): \SplFileInfo
{
return $this->_getFullPath($this->getHeader('check_nii_filename'));
}

/**
* Return a SPLFileInfo object based on this images's properties.
*
* @return \SplFileInfo
*/
public function getBvalFileInfo(): \SplFileInfo
{
return $this->_getFullPath($this->getHeader('check_bval_filename'));
}

/**
* Return a SPLFileInfo object based on this images's properties.
*
* @return \SplFileInfo
*/
public function getBvecFileInfo(): \SplFileInfo
{
return $this->_getFullPath($this->getHeader('check_bvec_filename'));
}

/**
* Return a SPLFileInfo object based on this images's properties.
*
* @return \SplFileInfo
*/
public function getBidsJsonFileInfo(): \SplFileInfo
{
return $this->_getFullPath($this->getHeader('bids_json_file'));
}

/**
* Return a SPLFileInfo object based on this images's thumbnail properties.
*
Expand Down