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

Remove filename from storageFiles #55

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Remove filename from storageFiles #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
acf4bcc
Remove filename from storageFiles
mrtcode May 25, 2017
58c089d
Small fixes
mrtcode May 31, 2017
3d84586
Remove 'duplicateFile'; remove S3 key prefixing depending on 'zip' fi…
mrtcode May 30, 2018
0d6b96b
Fix comment
mrtcode May 30, 2018
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
31 changes: 4 additions & 27 deletions controllers/ItemsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,7 +961,7 @@ private function _handleFileRequest($item) {
Zotero_DB::query("SET TRANSACTION ISOLATION LEVEL SERIALIZABLE");
Zotero_DB::beginTransaction();

// See if file exists with this filename
// See if file exists with this hash
$localInfo = Zotero_Storage::getLocalFileInfo($info);
if ($localInfo) {
$storageFileID = $localInfo['storageFileID'];
Expand All @@ -970,34 +970,11 @@ private function _handleFileRequest($item) {
if ($localInfo['size'] != $info->size) {
throw new Exception(
"Specified file size incorrect for existing file "
. $info->hash . "/" . $info->filename
. $info->hash
. " ({$localInfo['size']} != {$info->size})"
);
}
}
// If not found, see if there's a copy with a different name
else {
$oldStorageFileID = Zotero_Storage::getFileByHash($info->hash, $info->zip);
if ($oldStorageFileID) {
// Verify file size
$localInfo = Zotero_Storage::getFileInfoByID($oldStorageFileID);
if ($localInfo['size'] != $info->size) {
throw new Exception(
"Specified file size incorrect for duplicated file "
. $info->hash . "/" . $info->filename
. " ({$localInfo['size']} != {$info->size})"
);
}

// Create new file on S3 with new name
$storageFileID = Zotero_Storage::duplicateFile(
$oldStorageFileID,
$info->filename,
$info->zip,
$contentTypeHeader
);
}
}

// If we already have a file, add/update storageFileItems row and stop
if (!empty($storageFileID)) {
Expand Down Expand Up @@ -1102,13 +1079,13 @@ private function _handleFileRequest($item) {
else {
$remoteInfo = Zotero_Storage::getRemoteFileInfo($info);
if (!$remoteInfo) {
error_log("Remote file {$info->hash}/{$info->filename} not found");
error_log("Remote file {$info->hash} not found");
$this->e400("Remote file not found");
}
if ($remoteInfo->size != $info->size) {
error_log("Uploaded file size does not match "
. "({$remoteInfo->size} != {$info->size}) "
. "for file {$info->hash}/{$info->filename}");
. "for file {$info->hash}");
}
}

Expand Down
8 changes: 4 additions & 4 deletions model/Attachments.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ public static function getTemporaryURL(Zotero_Item $item, $localOnly=false) {

$info = Zotero_Storage::getLocalFileItemInfo($item);
$storageFileID = $info['storageFileID'];
$filename = $info['filename'];
$mtime = $info['mtime'];
$zip = $info['zip'];
$realFilename = preg_replace("/^storage:/", "", $item->attachmentPath);
Expand Down Expand Up @@ -188,16 +187,17 @@ public static function getTemporaryURL(Zotero_Item $item, $localOnly=false) {
if (!mkdir($downloadDir, 0777, true)) {
throw new Exception("Unable to create directory '$downloadDir'");
}
$archiveFilename = 'archive.zip';
if ($zip) {
$response = Zotero_Storage::downloadFile($info, $downloadDir);
$response = Zotero_Storage::downloadFile($info, $downloadDir, $archiveFilename);
}
else {
$response = Zotero_Storage::downloadFile($info, $downloadDir, $realFilename);
}
if ($response) {
if ($zip) {
$success = self::extractZip($downloadDir . $info['filename'], $dir);
Copy link
Member Author

Choose a reason for hiding this comment

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

If we create a random dir, I think there is no difference what filename the zip file has.

unlink($downloadDir . $info['filename']);
$success = self::extractZip($downloadDir . $archiveFilename, $dir);
unlink($downloadDir . $archiveFilename);
rmdir($downloadDir);

// Make sure charset is just a string with no spaces or newlines
Expand Down
167 changes: 63 additions & 104 deletions model/Storage.inc.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public static function getDownloadDetails($item) {
else {
return array(
'url' => $url,
'filename' => $info['filename'],
'size' => $info['size']
);
}
Expand Down Expand Up @@ -88,18 +87,22 @@ public static function getDownloadURL(Zotero_Item $item, $ttl=60) {
// http://docs.aws.amazon.com/aws-sdk-php/v3/api/api-s3-2006-03-01.html#headobject,
// but returning NotFound
if ($e->getAwsErrorCode() == 'NoSuchKey' || $e->getAwsErrorCode() == 'NotFound') {
// Try legacy key format, with zip flag and filename
// Try to find a legacy file: hash/filename or hash/c/filename
try {
$key = self::getPathPrefix($info['hash'], $info['zip']) . $info['filename'];
$s3Client->headObject([
$result = $s3Client->listObjects([
'Bucket' => Z_CONFIG::$S3_BUCKET,
'Key' => $key
'MaxKeys' => 1,
'Prefix' => $info['hash'],
]);
}
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey' || $e->getAwsErrorCode() == 'NotFound') {
$contents = $result->get('Contents');
if (!$contents || count($contents) < 1) {
Copy link
Member

Choose a reason for hiding this comment

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

What are the possible values for this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It returns all keys that are prefixed with a given hash. In our case:

hash
hash/file1
hash/c/file1

return false;
}

$key = $contents[0]['Key'];
}
catch (\Aws\S3\Exception\S3Exception $e) {
throw $e;
}
}
Expand All @@ -116,8 +119,7 @@ public static function getDownloadURL(Zotero_Item $item, $ttl=60) {
return (string) $s3Client->createPresignedRequest($cmd, "+$ttl seconds")->getUri();
}


public static function downloadFile(array $localFileItemInfo, $savePath, $filename=false) {
public static function downloadFile(array $localFileItemInfo, $savePath, $filename) {
if (!file_exists($savePath)) {
throw new Exception("Path '$savePath' does not exist");
}
Expand All @@ -131,24 +133,41 @@ public static function downloadFile(array $localFileItemInfo, $savePath, $filena
return $s3Client->getObject([
'Bucket' => Z_CONFIG::$S3_BUCKET,
'Key' => $localFileItemInfo['hash'],
'SaveAs' => $savePath . "/" . ($filename ? $filename : $localFileItemInfo['filename'])
'SaveAs' => $savePath . "/" . $filename
]);
}
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey') {
// Try legacy key format, with zip flag and filename
// Try to find a legacy file: hash/filename or hash/c/filename
try {
return $s3Client->getObject([
$result = $s3Client->listObjects([
'Bucket' => Z_CONFIG::$S3_BUCKET,
'Key' => self::getPathPrefix($localFileItemInfo['hash'], $localFileItemInfo['zip'])
. $localFileItemInfo['filename'],
'SaveAs' => $savePath . "/" . ($filename ? $filename : $localFileItemInfo['filename'])
'MaxKeys' => 1,
'Prefix' => $localFileItemInfo['hash'],
]);
}
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey') {
$contents = $result->get('Contents');
if (!$contents || count($contents) < 1) {
return false;
}
$key = $contents[0]['Key'];

try {
return $s3Client->getObject([
'Bucket' => Z_CONFIG::$S3_BUCKET,
'Key' => $key,
'SaveAs' => $savePath . "/" . $filename
]);
}
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey') {
return false;
}
throw $e;
}

}
catch (\Aws\S3\Exception\S3Exception $e) {
throw $e;
}
}
Expand All @@ -164,20 +183,18 @@ public static function logDownload($item, $downloadUserID, $ipAddress) {

$info = self::getLocalFileItemInfo($item);
$storageFileID = $info['storageFileID'];
$filename = $info['filename'];
$size = $info['size'];

$sql = "INSERT INTO storageDownloadLog
(ownerUserID, downloadUserID, ipAddress, storageFileID, filename, size)
VALUES (?, ?, INET_ATON(?), ?, ?, ?)";
(ownerUserID, downloadUserID, ipAddress, storageFileID, size)
VALUES (?, ?, INET_ATON(?), ?, ?)";
Zotero_DB::query(
$sql,
[
$ownerUserID,
$downloadUserID,
$ipAddress,
$storageFileID,
$filename,
$size
],
0,
Expand Down Expand Up @@ -267,16 +284,15 @@ public static function logUpload($uploadUserID, $item, $key, $ipAddress) {

$info = self::getLocalFileItemInfo($item);
$storageFileID = $info['storageFileID'];
$filename = $info['filename'];
$size = $info['size'];

$sql = "DELETE FROM storageUploadQueue WHERE uploadKey=?";
Zotero_DB::query($sql, $key);

$sql = "INSERT INTO storageUploadLog
(ownerUserID, uploadUserID, ipAddress, storageFileID, filename, size)
VALUES (?, ?, INET_ATON(?), ?, ?, ?)";
Zotero_DB::query($sql, array($ownerUserID, $uploadUserID, $ipAddress, $storageFileID, $filename, $size));
(ownerUserID, uploadUserID, ipAddress, storageFileID, size)
VALUES (?, ?, INET_ATON(?), ?, ?)";
Zotero_DB::query($sql, array($ownerUserID, $uploadUserID, $ipAddress, $storageFileID, $size));
}


Expand Down Expand Up @@ -392,72 +408,14 @@ public static function patchFile($item, $info, $algorithm, $patch) {
return $storageFileID;
}

public static function duplicateFile($storageFileID, $newName, $zip, $contentType=null) {
if (strlen($newName) == 0) {
throw new Exception("New name not provided");
}

$localInfo = self::getFileInfoByID($storageFileID);
if (!$localInfo) {
throw new Exception("File $storageFileID not found");
}

$s3Client = Z_Core::$AWS->createS3();
try {
$s3Client->headObject([
'Bucket' => Z_CONFIG::$S3_BUCKET,
'Key' => $localInfo['hash']
]);
}
// If file doesn't already exist named with just hash, copy it over
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey' || $e->getAwsErrorCode() == 'NotFound') {
try {
$s3Client->copyObject([
'Bucket' => Z_CONFIG::$S3_BUCKET,
'CopySource' => Z_CONFIG::$S3_BUCKET . '/'
. urlencode(self::getPathPrefix($localInfo['hash'], $localInfo['zip'])
. $localInfo['filename']),
'Key' => $localInfo['hash'],
'ACL' => 'private'
]);
}
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey') {
return false;
}
else {
throw $e;
}
}
}
else {
throw $e;
}
}

$info = new Zotero_StorageFileInfo;
foreach ($localInfo as $key => $val) {
$info->$key = $val;
}
$info->filename = $newName;
return self::addFile($info);
}


public static function getFileByHash($hash, $zip) {
$sql = "SELECT storageFileID FROM storageFiles WHERE hash=? AND zip=? LIMIT 1";
return Zotero_DB::valueQuery($sql, array($hash, (int) $zip));
}

public static function getFileInfoByID($storageFileID) {
$sql = "SELECT * FROM storageFiles WHERE storageFileID=?";
return Zotero_DB::rowQuery($sql, $storageFileID);
}

public static function getLocalFileInfo(Zotero_StorageFileInfo $info) {
$sql = "SELECT * FROM storageFiles WHERE hash=? AND filename=? AND zip=?";
return Zotero_DB::rowQuery($sql, array($info->hash, $info->filename, (int) $info->zip));
$sql = "SELECT * FROM storageFiles WHERE hash=? ORDER BY storageFileID LIMIT 1";
return Zotero_DB::rowQuery($sql, array($info->hash));
}

public static function getRemoteFileInfo(Zotero_StorageFileInfo $info) {
Expand All @@ -467,20 +425,26 @@ public static function getRemoteFileInfo(Zotero_StorageFileInfo $info) {
'Bucket' => Z_CONFIG::$S3_BUCKET,
'Key' => $info->hash
]);
$size = $result['ContentLength'];
}
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey' || $e->getAwsErrorCode() == 'NotFound') {
// Try legacy key format, with zip flag and filename
// Try to find a legacy file: hash/filename or hash/c/filename
try {
$result = $s3Client->headObject([
$result = $s3Client->listObjects([
'Bucket' => Z_CONFIG::$S3_BUCKET,
'Key' => self::getPathPrefix($info->hash, $info->zip) . $info->filename
'MaxKeys' => 1,
'Prefix' => $info->hash,
]);
}
catch (\Aws\S3\Exception\S3Exception $e) {
if ($e->getAwsErrorCode() == 'NoSuchKey' || $e->getAwsErrorCode() == 'NotFound') {
$contents = $result->get('Contents');
if (!$contents || count($contents) < 1) {
return false;
}

$size = $contents[0]['Size'];
}
catch (\Aws\S3\Exception\S3Exception $e) {
throw $e;
}
}
Expand All @@ -490,7 +454,7 @@ public static function getRemoteFileInfo(Zotero_StorageFileInfo $info) {
}

$storageFileInfo = new Zotero_StorageFileInfo;
$storageFileInfo->size = $result['ContentLength'];
$storageFileInfo->size = $size;

return $storageFileInfo;
}
Expand All @@ -517,12 +481,12 @@ public static function getLocalFileItemInfo($item) {
/**
* Get items associated with a unique file on S3
*/
public static function getFileItems($hash, $filename, $zip) {
public static function getFileItems($hash) {
throw new Exception("Unimplemented"); // would need to work across shards

$sql = "SELECT itemID FROM storageFiles JOIN storageFileItems USING (storageFileID)
WHERE hash=? AND filename=? AND zip=?";
$itemIDs = Zotero_DB::columnQuery($sql, array($hash, $filename, (int) $zip));
WHERE hash=?";
$itemIDs = Zotero_DB::columnQuery($sql, array($hash));
if (!$itemIDs) {
return array();
}
Expand All @@ -531,8 +495,8 @@ public static function getFileItems($hash, $filename, $zip) {


public static function addFile(Zotero_StorageFileInfo $info) {
$sql = "INSERT INTO storageFiles (hash, filename, size, zip) VALUES (?,?,?,?)";
return Zotero_DB::query($sql, array($info->hash, $info->filename, $info->size, (int) $info->zip));
$sql = "INSERT INTO storageFiles (hash, size, zip) VALUES (?,?,?)";
return Zotero_DB::query($sql, array($info->hash, $info->size, (int) $info->zip));
}


Expand Down Expand Up @@ -632,11 +596,6 @@ public static function deleteFileLibraryReference($storageFileID, $libraryID) {
}


public static function getPathPrefix($hash, $zip=false) {
return "$hash/" . ($zip ? "c/" : '');
}


public static function getUploadPOSTData($item, Zotero_StorageFileInfo $info) {
$params = self::generateUploadPOSTParams($item, $info);
$boundary = "---------------------------" . md5(uniqid());
Expand Down