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

[document_repository] Fixed multiple files with same name when uploading to document repository #3700

Closed
wants to merge 1 commit into from
Closed

[document_repository] Fixed multiple files with same name when uploading to document repository #3700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions SQL/0000-00-00-schema.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,7 @@ CREATE TABLE `document_repository` (
`EARLI` tinyint(1) DEFAULT '0',
`hide_video` tinyint(1) DEFAULT '0',
`File_category` int(3) unsigned DEFAULT NULL,
`UUID` varchar(36) DEFAULT '',
PRIMARY KEY (`record_id`),
KEY `fk_document_repository_1_idx` (`File_category`),
CONSTRAINT `fk_document_repository_1` FOREIGN KEY (`File_category`) REFERENCES `document_repository_categories` (`id`) ON DELETE NO ACTION ON UPDATE NO ACTION
Expand Down
1 change: 1 addition & 0 deletions SQL/Archive/19.2/2018-06-05_Add_UUID_column_to_document_repository.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ALTER TABLE document_repository ADD `UUID` varchar(36) DEFAULT '';
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe this patch should go into just the Archive directory? we're now on the 20 release. also, patches usually get moved to the version folders after a release if on minor

Copy link
Contributor

Choose a reason for hiding this comment

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

While we are at this, when would you submit the patches into the New_patches dir?

Copy link
Contributor

Choose a reason for hiding this comment

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

@um4r12 when you are on a branch that has these new directories. i'm going to bring this up in tomorrow's meeting :)

19 changes: 19 additions & 0 deletions modules/document_repository/ajax/documentDelete.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,23 @@
* @license http://www.gnu.org/licenses/gpl-3.0.txt GPLv3
* @link https://github.com/aces/Loris
*/

$user =& User::singleton();

if (!$user->hasPermission('document_repository_delete')) {
header("HTTP/1.1 403 Forbidden");
exit;
}

set_include_path(get_include_path().":../../project/libraries:../../php/libraries:");

require_once "NDB_Client.class.inc";
require_once "NDB_Config.class.inc";
require_once "Email.class.inc";

$client = new NDB_Client();
$client->initialize("../../project/config.xml");

$factory = NDB_Factory::singleton();
$baseURL = $factory->settings()->getBaseURL();

Expand Down Expand Up @@ -67,4 +72,18 @@
unlink($path);
}

// Cleanup empty directories
set_error_handler(
function () {
// Silence the E_WARNING when files exist in the directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if this is the best way to do this. Shouldn't we delete the files first in this case? I'm a little worried about this silencing useful warnings.

}
);
$rm_directory = __DIR__ . '/../user_uploads/'
. substr($dataDir, 0, strlen($dataDir)-(strlen($fileName)+1));
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you trying to chop off the filename from $dataDir here? If so, just use dirname();

rmdir($rm_directory);
$rm_directory = __DIR__ . '/../user_uploads/'
. $userName . '/' . $fileName;
rmdir($rm_directory);
restore_error_handler();

?>
84 changes: 69 additions & 15 deletions modules/document_repository/ajax/documentEditUpload.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,17 @@
require_once "NDB_Client.class.inc";
require_once "NDB_Config.class.inc";
require_once "Email.class.inc";

$client = new NDB_Client();
$client->initialize("../../project/config.xml");

$factory = NDB_Factory::singleton();
$baseURL = $factory->settings()->getBaseURL();

$config = NDB_Config::singleton();

// create Database object
$DB =& Database::singleton();
// Setup Database object.
$config =& \NDB_Config::singleton();
$db_config = $config->getSetting('database');
$db =& \Database::singleton();

$editNotifier = new NDB_Notifier(
"document_repository",
Expand All @@ -58,26 +60,61 @@
$instrument = $_POST['instrument'] !== '' ? $_POST['instrument'] : null;
$pscid = $_POST['pscid'] !== '' ? $_POST['pscid'] : null;
$visit = $_POST['visit'] !== '' ? $_POST['visit'] : null;
$comments = $_POST['comments'] !== '' ? $_POST['commnets'] : null;
$comments = $_POST['comments'] !== '' ? $_POST['comments'] : null;
$version = $_POST['version'] !== '' ? $_POST['version'] : null;
$uuid = uuid4();

$fileSize = $_FILES["file"]["size"];
$fileName = $_FILES["file"]["name"];
$fileType = end((explode(".", $fileName)));
$fileType = '';
// Handle retrieving the file type.
Copy link
Contributor

@johnsaigle johnsaigle Jul 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This is better done using pathinfo()

It's also more accurate to say that you are grabbing the uploaded file's path extension rather than its actual type. Properly getting the file type would involve looking at the file signatures.

if (preg_match('/\./', $fileName)) {
$pos = strrpos($fileName, '.', -1);
if ($pos+1 != strlen($fileName)) {
$fileType = substr(
$fileName,
strrpos($fileName, '.', -1)+1
);
}
}
$sql_statement = $db->prepare(
'SELECT File_name, version FROM document_repository '
.'WHERE File_name=? AND uploaded_by=?'
);
$sql_statement->bindParam(1, $fileName, PDO::PARAM_STR);
$sql_statement->bindParam(2, $puser, PDO::PARAM_STR);
$sql_statement->execute();
$sql_result = $sql_statement->fetchAll(PDO::FETCH_ASSOC);

// __DIR__ is the document_repository ajax directory
// when this script is executing. Go up a level to the
// document_repository module directory, and use a
// user_uploads directory as a base for user uploads
$base_path = __DIR__ . "/../user_uploads/";
$fileBase = $puser . "/" . $fileName;
$base_path = realpath(__DIR__ . '/..') . '/user_uploads/';
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you mind changing this to $upload_path? The "base path" in LORIS typically refers to the Config setting corresponding to the web root.

$fileBase = $puser . '/'
. $fileName
. '/' . $uuid
Copy link
Contributor

Choose a reason for hiding this comment

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

Please put all the front-slashes at the front of the line or the back of the line for consistency.

. '/' . $fileName;
Copy link
Contributor

Choose a reason for hiding this comment

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

$filename is used twice. Is it supposed to be like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the the first $fileName is referring to the directory, whereas the second one is referring to the actual file.
ex (from above). user_uploads/admin/text.png/{uuid}/text.png

Copy link
Member Author

Choose a reason for hiding this comment

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

@PapillonMcGill um4r12 is correct and so it's supposed to be like that.


// Create user directory /base_path/user
if (!file_exists($base_path . $puser)) {
mkdir($base_path . $puser, 0777);
mkdir($base_path . $puser, 0770);
}


$target_path = $base_path . $fileBase;
// Create filename directory /base_path/user/fileName
if (!file_exists($base_path . $puser . '/' . $fileName)) {
mkdir($base_path . $puser . '/' . $fileName, 0770);
}
// Create uuid directory /base_path/user/fileName/uuid
if (!file_exists(
$base_path . $puser . '/' . $fileName . '/' . $uuid
)
) {
mkdir(
$base_path . $puser . '/' . $fileName . '/' . $uuid,
0770
);
}
$target_path = $base_path . $fileBase;

if (move_uploaded_file($_FILES["file"]["tmp_name"], $target_path)) {
$success = $DB->insert(
Copy link
Contributor

@Jkat Jkat Aug 6, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

$db (didn't check out the code, just noticing you changed from $DB to $db)

Copy link
Contributor

Choose a reason for hiding this comment

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

(and same with line 174 & 176)

Expand All @@ -95,10 +132,10 @@
'PSCID' => $pscid,
'visitLabel' => $visit,
'File_type' => $fileType,
'UUID' => $uuid,
)
);
$msg_data['newDocument']
= $baseURL . "/document_repository/";
$msg_data['newDocument'] = $baseURL . "/document_repository/";
$msg_data['document'] = $fileName;

$uploadNotifier->notify($msg_data);
Expand All @@ -110,7 +147,7 @@
} else {
echo "There was an error uploading the file";
}
} elseif ($action == 'edit') {
} else if ($action == 'edit') {
$id = $_POST['idEdit'];
$category = $_POST['categoryEdit'];
$instrument = $_POST['instrumentEdit'];
Expand Down Expand Up @@ -147,4 +184,21 @@
}
}

/**
* Create a UUID v4 string.
*
* Source from comments:
* http://php.net/manual/en/function.com-create-guid.php
* Maybe move to Utilities class.
*
* @return String $version
*/
function uuid4()
{
$data = openssl_random_pseudo_bytes(16);
$data[6] = chr(ord($data[6]) & 0x0f | 0x40); // set version to 0100
$data[8] = chr(ord($data[8]) & 0x3f | 0x80); // set bits 6-7 to 10
return vsprintf('%s%s-%s-%s-%s-%s%s%s', str_split(bin2hex($data), 4));
}

?>
1 change: 1 addition & 0 deletions modules/document_repository/test/document_repositoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public function setUp()
'EARLI' => '0',
'hide_video' => '0',
'File_category' => '9999999',
'UUID' => 'dc987d5c-6e5d-11e8-adc0-fa7ae01bbebc',
)
);

Expand Down
2 changes: 1 addition & 1 deletion test/RBdata.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1673,7 +1673,7 @@ UNLOCK TABLES;
LOCK TABLES `document_repository` WRITE;
DELETE FROM `document_repository`;
/*!40000 ALTER TABLE `document_repository` DISABLE KEYS */;
INSERT INTO `document_repository` VALUES (1,'','','',NULL,'2016-07-27 18:00:10','admin/bread-breakfast-knife.jpg','bread-breakfast-knife.jpg','jpg','1',439412,'admin',2,' ',NULL,0,0,2),(2,'','','',NULL,'2016-07-27 18:00:47','admin/bread-food-healthy-breakfast.jpg','bread-food-healthy-breakfast.jpg','jpg','2',1336341,'admin',2,' ',NULL,0,0,2),(3,'','','',NULL,'2016-07-27 18:02:58','admin/bread-food-healthy-breakfast.jpg','bread-food-healthy-breakfast.jpg','jpg','1',1336341,'admin',2,' ',NULL,0,0,2),(4,'','','',NULL,'2016-07-27 18:04:00','admin/test.txt','test.txt','txt','',29,'admin',3,' ',NULL,0,0,1),(5,'','','',NULL,'2016-07-27 18:04:53','admin/test.pdf','test.pdf','pdf','',7581,'admin',4,' ',NULL,0,0,3),(6,'','radiology_review','',NULL,'2016-07-27 18:05:36','admin/test.pdf','test.pdf','pdf','',7581,'admin',2,' ',NULL,0,0,3),(7,'','medical_history','',NULL,'2016-07-27 18:05:59','admin/test.pdf','test.pdf','pdf','',7581,'admin',3,' ',NULL,0,0,3),(8,'MTL003','bmi','V1',NULL,'2016-07-27 18:06:43','admin/test.txt','test.txt','txt','',29,'admin',2,' ',NULL,0,0,1);
INSERT INTO `document_repository` VALUES (1,'','','',NULL,'2016-07-27 18:00:10','admin/bread-breakfast-knife.jpg','bread-breakfast-knife.jpg','jpg','1',439412,'admin',2,' ',NULL,0,0,2,'531cc990-6e6f-11e8-adc0-fa7ae01bbebc'),(2,'','','',NULL,'2016-07-27 18:00:47','admin/bread-food-healthy-breakfast.jpg','bread-food-healthy-breakfast.jpg','jpg','2',1336341,'admin',2,' ',NULL,0,0,2,'736c6782-6e6f-11e8-adc0-fa7ae01bbebc'),(3,'','','',NULL,'2016-07-27 18:02:58','admin/bread-food-healthy-breakfast.jpg','bread-food-healthy-breakfast.jpg','jpg','1',1336341,'admin',2,' ',NULL,0,0,2,'876707d8-6e6f-11e8-adc0-fa7ae01bbebc'),(4,'','','',NULL,'2016-07-27 18:04:00','admin/test.txt','test.txt','txt','',29,'admin',3,' ',NULL,0,0,1,'917846f6-6e6f-11e8-adc0-fa7ae01bbebc'),(5,'','','',NULL,'2016-07-27 18:04:53','admin/test.pdf','test.pdf','pdf','',7581,'admin',4,' ',NULL,0,0,3,'99e04686-6e6f-11e8-adc0-fa7ae01bbebc'),(6,'','radiology_review','',NULL,'2016-07-27 18:05:36','admin/test.pdf','test.pdf','pdf','',7581,'admin',2,' ',NULL,0,0,3,'a256481a-6e6f-11e8-adc0-fa7ae01bbebc'),(7,'','medical_history','',NULL,'2016-07-27 18:05:59','admin/test.pdf','test.pdf','pdf','',7581,'admin',3,' ',NULL,0,0,3,'ab3e9a9a-6e6f-11e8-adc0-fa7ae01bbebc'),(8,'MTL003','bmi','V1',NULL,'2016-07-27 18:06:43','admin/test.txt','test.txt','txt','',29,'admin',2,' ',NULL,0,0,1,'b2b48a28-6e6f-11e8-adc0-fa7ae01bbebc');
/*!40000 ALTER TABLE `document_repository` ENABLE KEYS */;
UNLOCK TABLES;

Expand Down