Skip to content
Snippets Groups Projects
Commit 00a1371d authored by Jared Hancock's avatar Jared Hancock
Browse files

files: Attempt to standardize thread entry attaching 

This commit attempts to remove some of the confusing and redundant code to
attach files to thread entries and replace it with a single code base. It
also attempts to remove and error where a single attachment might be
attached to a new thread entry multiple times.

Lastly, it removes the insert followed by an update for emails with inline
images. This should improve performance processing emails as only one trip
to the database is now necessary for thread entries with inline images.
parent 5acdf568
Branches
Tags
No related merge requests found
Hide whitespace changes
Inline Side-by-side
include/class.thread.php
+ 117
− 144
View file @ 00a1371d
...@@ -881,98 +881,73 @@ implements TemplateVariable { ...@@ -881,98 +881,73 @@ implements TemplateVariable {
return $this->hasFlag(self::FLAG_SYSTEM); return $this->hasFlag(self::FLAG_SYSTEM);
} }
//Web uploads - caller is expected to format, validate and set any errors. protected function normalizeFileInfo($files, $add_error=true) {
function uploadFiles($files) { static $error_descriptions = array(
UPLOAD_ERR_INI_SIZE => /* @trans */ 'File is too large',
if(!$files || !is_array($files)) UPLOAD_ERR_FORM_SIZE => /* @trans */ 'File is too large',
return false; UPLOAD_ERR_PARTIAL => 'The uploaded file was only partially uploaded.',
UPLOAD_ERR_NO_TMP_DIR => 'Missing a temporary folder.',
UPLOAD_ERR_CANT_WRITE => 'Failed to write file to disk.',
UPLOAD_ERR_EXTENSION => 'A PHP extension stopped the file upload.',
);
$uploaded=array(); if (!is_array($files))
foreach($files as $file) { $files = array($files);
if($file['error'] && $file['error']==UPLOAD_ERR_NO_FILE)
$ids = array();
foreach ($files as $name => $file) {
$F = array('inline' => is_array($file) && @$file['inline']);
if (is_numeric($file))
$fileId = $file;
elseif ($file instanceof AttachmentFile)
$fileId = $file->getId();
elseif (is_array($file) && isset($file['id']))
$fileId = $file['id'];
elseif ($AF = AttachmentFile::create($file))
$fileId = $AF->getId();
elseif ($add_error) {
$error = $file['error']
?: sprintf(_S('Unable to import attachment - %s'),
$name ?: $file['name']);
if (is_numeric($error) && isset($error_descriptions[$error])) {
$error = sprintf(_S('Error #%1$d: %2$s'), $error,
_S($error_descriptions[$error]));
}
// No need to log the missing-file error number
if ($error != UPLOAD_ERR_NO_FILE)
$this->getThread()->getObject()->logNote(
_S('File Import Error'), $error, _S('SYSTEM'), false);
continue; continue;
if(!$file['error']
&& ($F=AttachmentFile::upload($file))
&& $this->saveAttachment($F))
$uploaded[]= $F->getId();
else {
if(!$file['error'])
$error = sprintf(__('Unable to upload file - %s'),$file['name']);
elseif(is_numeric($file['error']))
$error ='Error #'.$file['error']; //TODO: Transplate to string.
else
$error = $file['error'];
/*
Log the error as an internal note.
XXX: We're doing it here because it will eventually become a thread post comment (hint: comments coming!)
XXX: logNote must watch for possible loops
*/
$this->getThread()->getObject()->logNote(__('File Upload Error'), $error, 'SYSTEM', false);
} }
} $F['id'] = $fileId;
return $uploaded;
}
function importAttachments(&$attachments) {
if(!$attachments || !is_array($attachments)) if (is_string($name))
return null; $F['name'] = $name;
if (isset($AF))
$files = array(); $F['file'] = $AF;
foreach($attachments as &$attachment)
if(($id=$this->importAttachment($attachment)))
$files[] = $id;
return $files;
}
/* Emailed & API attachments handler */
function importAttachment(&$attachment) {
if(!$attachment || !is_array($attachment)) // Add things like the `key` field, but don't change current
return null; // keys of the file array
if (is_array($file))
$F += $file;
$A=null; $ids[] = $F;
if ($attachment['error'] || !($A=$this->saveAttachment($attachment))) {
$error = $attachment['error'];
if(!$error)
$error = sprintf(_S('Unable to import attachment - %s'),
$attachment['name']);
//FIXME: logComment here
$this->getThread()->getObject()->logNote(
_S('File Import Error'), $error, _S('SYSTEM'), false);
} }
return $ids;
return $A;
} }
/* /*
Save attachment to the DB. Save attachment to the DB.
@file is a mixed var - can be ID or file hashtable. @file is a mixed var - can be ID or file hashtable.
*/ */
function saveAttachment(&$file, $name=false) { function createAttachment($file, $name=false) {
$inline = is_array($file) && @$file['inline'];
if (is_numeric($file))
$fileId = $file;
elseif ($file instanceof AttachmentFile)
$fileId = $file->getId();
elseif ($F = AttachmentFile::create($file))
$fileId = $F->getId();
elseif (is_array($file) && isset($file['id']))
$fileId = $file['id'];
else
return false;
$att = new Attachment(array( $att = new Attachment(array(
'type' => 'H', 'type' => 'H',
'object_id' => $this->getId(), 'object_id' => $this->getId(),
'file_id' => $fileId, 'file_id' => $file['id'],
'inline' => $inline ? 1 : 0, 'inline' => $file['inline'] ? 1 : 0,
)); ));
// Record varying file names in the attachment record // Record varying file names in the attachment record
...@@ -984,7 +959,7 @@ implements TemplateVariable { ...@@ -984,7 +959,7 @@ implements TemplateVariable {
} }
if ($filename) { if ($filename) {
// This should be a noop since the ORM caches on PK // This should be a noop since the ORM caches on PK
$F = $F ?: AttachmentFile::lookup($fileId); $F = @$file['file'] ?: AttachmentFile::lookup($file['id']);
// XXX: This is not Unicode safe // XXX: This is not Unicode safe
if ($F && 0 !== strcasecmp($F->name, $filename)) if ($F && 0 !== strcasecmp($F->name, $filename))
$att->name = $filename; $att->name = $filename;
...@@ -995,13 +970,11 @@ implements TemplateVariable { ...@@ -995,13 +970,11 @@ implements TemplateVariable {
return $att; return $att;
} }
function saveAttachments($files) { function createAttachments(array $files) {
$attachments = array(); foreach ($files as $info) {
foreach ($files as $name=>$file) { if ($A = $this->createAttachment($info, @$info['name'] ?: false))
if (($A = $this->saveAttachment($file, $name)))
$attachments[] = $A; $attachments[] = $A;
} }
return $attachments; return $attachments;
} }
...@@ -1326,29 +1299,6 @@ implements TemplateVariable { ...@@ -1326,29 +1299,6 @@ implements TemplateVariable {
$vars['body'] = new TextThreadEntryBody($vars['body']); $vars['body'] = new TextThreadEntryBody($vars['body']);
} }
// Drop stripped images
if ($vars['attachments']) {
foreach ($vars['body']->getStrippedImages() as $cid) {
foreach ($vars['attachments'] as $i=>$a) {
if (@$a['cid'] && $a['cid'] == $cid) {
// Inline referenced attachment was stripped
unset($vars['attachments'][$i]);
}
}
}
}
// Handle extracted embedded images (<img src="data:base64,..." />).
// The extraction has already been performed in the ThreadEntryBody
// class. Here they should simply be added to the attachments list
if ($atts = $vars['body']->getEmbeddedHtmlImages()) {
if (!is_array($vars['attachments']))
$vars['attachments'] = array();
foreach ($atts as $info) {
$vars['attachments'][] = $info;
}
}
if (!($body = $vars['body']->getClean())) if (!($body = $vars['body']->getClean()))
$body = '-'; //Special tag used to signify empty message as stored. $body = '-'; //Special tag used to signify empty message as stored.
...@@ -1377,11 +1327,6 @@ implements TemplateVariable { ...@@ -1377,11 +1327,6 @@ implements TemplateVariable {
if (!($vars['staffId'] || $vars['userId'])) if (!($vars['staffId'] || $vars['userId']))
$entry->flags |= self::FLAG_SYSTEM; $entry->flags |= self::FLAG_SYSTEM;
if (!isset($vars['attachments']) || !$vars['attachments'])
// Otherwise, body will be configured in a block below (after
// inline attachments are saved and updated in the database)
$entry->body = $body;
if (isset($vars['pid'])) if (isset($vars['pid']))
$entry->pid = $vars['pid']; $entry->pid = $vars['pid'];
// Check if 'reply_to' is in the $vars as the previous ThreadEntry // Check if 'reply_to' is in the $vars as the previous ThreadEntry
...@@ -1394,51 +1339,79 @@ implements TemplateVariable { ...@@ -1394,51 +1339,79 @@ implements TemplateVariable {
if ($vars['ip_address']) if ($vars['ip_address'])
$entry->ip_address = $vars['ip_address']; $entry->ip_address = $vars['ip_address'];
if (!$entry->save())
return false;
/************* ATTACHMENTS *****************/ /************* ATTACHMENTS *****************/
// Drop stripped email inline images
if ($vars['attachments']) {
foreach ($vars['body']->getStrippedImages() as $cid) {
foreach ($vars['attachments'] as $i=>$a) {
if (@$a['cid'] && $a['cid'] == $cid) {
// Inline referenced attachment was stripped
unset($vars['attachments'][$i]);
}
}
}
}
//Upload/save attachments IF ANY // Handle extracted embedded images (<img src="data:base64,..." />).
if($vars['files']) //expects well formatted and VALIDATED files array. // The extraction has already been performed in the ThreadEntryBody
$entry->uploadFiles($vars['files']); // class. Here they should simply be added to the attachments list
if ($atts = $vars['body']->getEmbeddedHtmlImages()) {
//Canned attachments... if (!is_array($vars['attachments']))
if($vars['cannedattachments'] && is_array($vars['cannedattachments'])) $vars['attachments'] = array();
$entry->saveAttachments($vars['cannedattachments']); foreach ($atts as $info) {
$vars['attachments'][] = $info;
//Emailed or API attachments }
if (isset($vars['attachments']) && $vars['attachments']) { }
foreach ($vars['attachments'] as &$a)
if (isset($a['cid']) && $a['cid'] $attached_files = array();
&& strpos($body, 'cid:'.$a['cid']) !== false) foreach (array(
$a['inline'] = true; // Web uploads and canned attachments
unset($a); $vars['files'], $vars['cannedattachments'],
// Emailed or API attachments
$entry->importAttachments($vars['attachments']); $vars['attachments'],
foreach ($vars['attachments'] as $a) { // Inline images (attached to the draft)
// Change <img src="cid:"> inside the message to point to Draft::getAttachmentIds($body),
// a unique hash-code for the attachment. Since the ) as $files
// content-id will be discarded, only the unique hash-code ) {
// will be available to retrieve the image later if (is_array($files)) {
if ($a['cid'] && $a['key']) { // Detect *inline* email attachments
$body = preg_replace('/src=("|\'|\b)(?:cid:)?' foreach ($files as $i=>$a) {
. preg_quote($a['cid'], '/').'\1/i', if (isset($a['cid']) && $a['cid']
'src="cid:'.$a['key'].'"', $body); && strpos($body, 'cid:'.$a['cid']) !== false)
$files[$i]['inline'] = true;
}
foreach ($entry->normalizeFileInfo($files) as $F) {
// Deduplicate on the `key` attribute
$attached_files[$F['key']] = $F;
} }
} }
}
$entry->body = $body; // Change <img src="cid:"> inside the message to point to a unique
if (!$entry->save()) // hash-code for the attachment. Since the content-id will be
return false; // discarded, only the unique hash-code (key) will be available to
// retrieve the image later
foreach ($attached_files as $key => $a) {
if (isset($a['cid']) && $a['cid']) {
$body = preg_replace('/src=("|\'|\b)(?:cid:)?'
. preg_quote($a['cid'], '/').'\1/i',
'src="cid:'.$key.'"', $body);
}
} }
// Set body here after it was rewritten to capture the stored file
// keys (above)
$entry->body = $body;
if (!$entry->save())
return false;
// Associate the attached files with this new entry
$entry->createAttachments($attached_files);
// Save mail message id, if available // Save mail message id, if available
$entry->saveEmailInfo($vars); $entry->saveEmailInfo($vars);
// Inline images (attached to the draft)
$entry->saveAttachments(Draft::getAttachmentIds($body));
Signal::send('threadentry.created', $entry); Signal::send('threadentry.created', $entry);
return $entry; return $entry;
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment