From ed4fcc9011313ab62741849670c78607e75ed56f Mon Sep 17 00:00:00 2001 From: James Cole Date: Sat, 18 Jul 2015 22:17:31 +0200 Subject: [PATCH] Some optimisation. --- app/Helpers/Attachments/AttachmentHelper.php | 75 ++++++++++------ config/firefly.php | 2 + resources/lang/en/validation.php | 92 ++++++++++---------- 3 files changed, 98 insertions(+), 71 deletions(-) diff --git a/app/Helpers/Attachments/AttachmentHelper.php b/app/Helpers/Attachments/AttachmentHelper.php index 4af6557a13..3b93eaa24a 100644 --- a/app/Helpers/Attachments/AttachmentHelper.php +++ b/app/Helpers/Attachments/AttachmentHelper.php @@ -3,6 +3,7 @@ namespace FireflyIII\Helpers\Attachments; use Auth; +use Config; use Crypt; use FireflyIII\Models\Attachment; use Illuminate\Database\Eloquent\Model; @@ -18,11 +19,13 @@ use Symfony\Component\HttpFoundation\File\UploadedFile; class AttachmentHelper implements AttachmentHelperInterface { - // move to config: - protected $maxUploadSize = 1048576; // 1MB per file - protected $allowedMimes = ['image/png', 'image/jpeg', 'application/pdf']; - + /** @var int */ + protected $maxUploadSize; + /** @var array */ + protected $allowedMimes; + /** @var MessageBag */ public $errors; + /** @var MessageBag */ public $messages; /** @@ -30,14 +33,16 @@ class AttachmentHelper implements AttachmentHelperInterface */ public function __construct() { - $this->errors = new MessageBag; - $this->messages = new MessageBag; + $this->maxUploadSize = Config::get('firefly.maxUploadSize'); + $this->allowedMimes = Config::get('firefly.allowedMimes'); + $this->errors = new MessageBag; + $this->messages = new MessageBag; } /** * @param Attachment $attachment * - * @return mixed + * @return string */ public function getAttachmentLocation(Attachment $attachment) { @@ -78,8 +83,8 @@ class AttachmentHelper implements AttachmentHelperInterface $count = Auth::user()->attachments()->where('md5', $md5)->where('attachable_id', $model->id)->where('attachable_type', $class)->count(); if ($count > 0) { - $err = 'File ' . e($name) . ' already attached to this object.'; - $this->errors->add('attachments', $err); + $msg = trans('validation.file_already_attached', ['name' => $name]); + $this->errors->add('attachments', $msg); return true; } @@ -89,8 +94,11 @@ class AttachmentHelper implements AttachmentHelperInterface /** * @param UploadedFile $file + * @param Model $model + * + * @return bool */ - protected function processFile(UploadedFile $file, Model $model) + protected function validateUpload(UploadedFile $file, Model $model) { if (!$this->validMime($file)) { return false; @@ -102,8 +110,23 @@ class AttachmentHelper implements AttachmentHelperInterface return false; } - // create Attachment object. - $attachment = new Attachment; + return true; + } + + /** + * @param UploadedFile $file + * @param Model $model + * + * @return bool|Attachment + */ + protected function processFile(UploadedFile $file, Model $model) + { + $validation = $this->validateUpload($file, $model); + if ($validation === false) { + return false; + } + + $attachment = new Attachment; // create Attachment object. $attachment->user()->associate(Auth::user()); $attachment->attachable()->associate($model); $attachment->md5 = md5_file($file->getRealPath()); @@ -113,23 +136,22 @@ class AttachmentHelper implements AttachmentHelperInterface $attachment->uploaded = 0; $attachment->save(); - // encrypt and move file to storage. - $path = $file->getRealPath(); + $path = $file->getRealPath(); // encrypt and move file to storage. $content = file_get_contents($path); $encrypted = Crypt::encrypt($content); // store it: - $upload = storage_path('upload') . DIRECTORY_SEPARATOR . 'at-' . $attachment->id . '.data'; + $upload = $this->getAttachmentLocation($attachment); if (is_writable(dirname($upload))) { file_put_contents($upload, $encrypted); } - // update Attacment. - $attachment->uploaded = 1; + $attachment->uploaded = 1; // update attachment $attachment->save(); - // add message: - $this->messages->add('attachments', 'File ' . e($file->getClientOriginalName()) . ' was uploaded successfully.'); + $name = e($file->getClientOriginalName()); // add message: + $msg = trans('validation.file_attached', ['name' => $name]); + $this->messages->add('attachments', $msg); // return it. return $attachment; @@ -144,12 +166,12 @@ class AttachmentHelper implements AttachmentHelperInterface */ protected function validMime(UploadedFile $file) { - $mime = $file->getMimeType(); - $name = $file->getClientOriginalName(); + $mime = e($file->getMimeType()); + $name = e($file->getClientOriginalName()); if (!in_array($mime, $this->allowedMimes)) { - $err = 'File ' . e($name) . ' is of type ' . e($mime) . '.'; - $this->errors->add('attachments', $err); + $msg = trans('validation.file_invalid_mime', ['name' => $name, 'mime' => $mime]); + $this->errors->add('attachments', $msg); return false; } @@ -165,11 +187,10 @@ class AttachmentHelper implements AttachmentHelperInterface protected function validSize(UploadedFile $file) { $size = $file->getSize(); - $name = $file->getClientOriginalName(); + $name = e($file->getClientOriginalName()); if ($size > $this->maxUploadSize) { - - $err = 'File ' . e($name) . ' is too large.'; - $this->errors->add('attachments', $err); + $msg = trans('validation.file_too_large', ['name' => $name]); + $this->errors->add('attachments', $msg); return false; } diff --git a/config/firefly.php b/config/firefly.php index a2733e3ef5..fb8286fa68 100644 --- a/config/firefly.php +++ b/config/firefly.php @@ -6,6 +6,8 @@ return [ 'index_periods' => ['1D', '1W', '1M', '3M', '6M', '1Y', 'custom'], 'budget_periods' => ['daily', 'weekly', 'monthly', 'quarterly', 'half-year', 'yearly'], 'csv_import_enabled' => true, + 'maxUploadSize' => 5242880, + 'allowedMimes' => ['image/png', 'image/jpeg', 'application/pdf'], 'piggy_bank_periods' => [ 'week' => 'Week', 'month' => 'Month', diff --git a/resources/lang/en/validation.php b/resources/lang/en/validation.php index e2dae604a2..9aede79f7d 100644 --- a/resources/lang/en/validation.php +++ b/resources/lang/en/validation.php @@ -13,69 +13,73 @@ return [ | */ - "accepted" => "The :attribute must be accepted.", - "active_url" => "The :attribute is not a valid URL.", - "after" => "The :attribute must be a date after :date.", - "alpha" => "The :attribute may only contain letters.", - "alpha_dash" => "The :attribute may only contain letters, numbers, and dashes.", - "alpha_num" => "The :attribute may only contain letters and numbers.", - "array" => "The :attribute must be an array.", - "unique_for_user" => "There already is an entry with this :attribute.", - "before" => "The :attribute must be a date before :date.", - 'unique_object_for_user' => 'This name is already in use', + 'file_already_attached' => 'Uploaded file ":name" is already attached to this object.', + 'file_attached' => 'Succesfully uploaded file ":name".', + 'file_invalid_mime' => 'File ":name" is of type ":mime" which is not accepted as a new upload.', + 'file_too_large' => 'File ":name" is too large.', + "accepted" => "The :attribute must be accepted.", + "active_url" => "The :attribute is not a valid URL.", + "after" => "The :attribute must be a date after :date.", + "alpha" => "The :attribute may only contain letters.", + "alpha_dash" => "The :attribute may only contain letters, numbers, and dashes.", + "alpha_num" => "The :attribute may only contain letters and numbers.", + "array" => "The :attribute must be an array.", + "unique_for_user" => "There already is an entry with this :attribute.", + "before" => "The :attribute must be a date before :date.", + 'unique_object_for_user' => 'This name is already in use', 'unique_account_for_user' => 'This account name is already in use', - "between" => [ + "between" => [ "numeric" => "The :attribute must be between :min and :max.", "file" => "The :attribute must be between :min and :max kilobytes.", "string" => "The :attribute must be between :min and :max characters.", "array" => "The :attribute must have between :min and :max items.", ], - "boolean" => "The :attribute field must be true or false.", - "confirmed" => "The :attribute confirmation does not match.", - "date" => "The :attribute is not a valid date.", - "date_format" => "The :attribute does not match the format :format.", - "different" => "The :attribute and :other must be different.", - "digits" => "The :attribute must be :digits digits.", - "digits_between" => "The :attribute must be between :min and :max digits.", - "email" => "The :attribute must be a valid email address.", - "filled" => "The :attribute field is required.", - "exists" => "The selected :attribute is invalid.", - "image" => "The :attribute must be an image.", - "in" => "The selected :attribute is invalid.", - "integer" => "The :attribute must be an integer.", - "ip" => "The :attribute must be a valid IP address.", - "max" => [ + "boolean" => "The :attribute field must be true or false.", + "confirmed" => "The :attribute confirmation does not match.", + "date" => "The :attribute is not a valid date.", + "date_format" => "The :attribute does not match the format :format.", + "different" => "The :attribute and :other must be different.", + "digits" => "The :attribute must be :digits digits.", + "digits_between" => "The :attribute must be between :min and :max digits.", + "email" => "The :attribute must be a valid email address.", + "filled" => "The :attribute field is required.", + "exists" => "The selected :attribute is invalid.", + "image" => "The :attribute must be an image.", + "in" => "The selected :attribute is invalid.", + "integer" => "The :attribute must be an integer.", + "ip" => "The :attribute must be a valid IP address.", + "max" => [ "numeric" => "The :attribute may not be greater than :max.", "file" => "The :attribute may not be greater than :max kilobytes.", "string" => "The :attribute may not be greater than :max characters.", "array" => "The :attribute may not have more than :max items.", ], - "mimes" => "The :attribute must be a file of type: :values.", - "min" => [ + "mimes" => "The :attribute must be a file of type: :values.", + "min" => [ "numeric" => "The :attribute must be at least :min.", "file" => "The :attribute must be at least :min kilobytes.", "string" => "The :attribute must be at least :min characters.", "array" => "The :attribute must have at least :min items.", ], - "not_in" => "The selected :attribute is invalid.", - "numeric" => "The :attribute must be a number.", - "regex" => "The :attribute format is invalid.", - "required" => "The :attribute field is required.", - "required_if" => "The :attribute field is required when :other is :value.", - "required_with" => "The :attribute field is required when :values is present.", - "required_with_all" => "The :attribute field is required when :values is present.", - "required_without" => "The :attribute field is required when :values is not present.", - "required_without_all" => "The :attribute field is required when none of :values are present.", - "same" => "The :attribute and :other must match.", - "size" => [ + "not_in" => "The selected :attribute is invalid.", + "numeric" => "The :attribute must be a number.", + "regex" => "The :attribute format is invalid.", + "required" => "The :attribute field is required.", + "required_if" => "The :attribute field is required when :other is :value.", + "required_with" => "The :attribute field is required when :values is present.", + "required_with_all" => "The :attribute field is required when :values is present.", + "required_without" => "The :attribute field is required when :values is not present.", + "required_without_all" => "The :attribute field is required when none of :values are present.", + "same" => "The :attribute and :other must match.", + "size" => [ "numeric" => "The :attribute must be :size.", "file" => "The :attribute must be :size kilobytes.", "string" => "The :attribute must be :size characters.", "array" => "The :attribute must contain :size items.", ], - "unique" => "The :attribute has already been taken.", - "url" => "The :attribute format is invalid.", - "timezone" => "The :attribute must be a valid zone.", + "unique" => "The :attribute has already been taken.", + "url" => "The :attribute format is invalid.", + "timezone" => "The :attribute must be a valid zone.", /* |-------------------------------------------------------------------------- @@ -88,7 +92,7 @@ return [ | */ - 'custom' => [ + 'custom' => [ 'attribute-name' => [ 'rule-name' => 'custom-message', ], @@ -105,6 +109,6 @@ return [ | */ - 'attributes' => [], + 'attributes' => [], ];