правильно ли я подхожу к написанию Uploader-a

Духовность™

Продвинутый новичок
правильно ли я подхожу к написанию Uploader-a

Пишу класс для загрузки файлов на сайт. Покритикуйте интерфейс, что так а что не так:

PHP:
class Base_Upload
{
    /**
     * Принимает значение одного элемента массива $_FILES 
     * 
     * @param array 
     */
    public function __construct(&$file);

    /**
     * Устанавливает будущее имя загружаемого файла.
     * Если имя не указывается, файл будет сохранён с оригинальным именем.
     *
     * @param string
     */
    public function setName($file_name);

    /**
     * Устанавливает расширение загружаемого файла.
     * Если расширение не указывается, файл будет сохранён с оригинальным расширением.
     *
     * @param string
     */
    public function setExt($file_ext);

    /**
     * Устанавливает максимально-дрпустимый размер файла.
     * Значение $size может быть любой формой представления
     * человекопонятной нумерации размерности данных, принятых в PHP: 8M, 2B, 30G
     *
     * @param string
     * @return void
     */
    public function setMaxSize($size);

    /**
     * Устанавливает допустимые mime-типы загружаемых файлов.
     *
     * @param array|string массив или строка - допустимые mime-типы
     * @return void
     */
    public function setAllowableType($type);

    /**
     * Возвращает ошибки, если таковые имеются.
     *
     * @return void|array
     */
    public function getErrors();

    /**
     * Копирует загруженный файл в директорию $directory
     * 
     * @param $directory
     */
    public function copy($directory);
}



if (Http_Request::isPost())
{
    if (isset($_FILES['my']))
    {
        foreach ($_FILES['my']['name'] as $id => $__)
        {
            $file = array
            (
                'name'     => &$_FILES['my']['name'][$id],
                'type'     => &$_FILES['my']['type'][$id],
                'tmp_name' => &$_FILES['my']['tmp_name'][$id],
                'error'    => &$_FILES['my']['error'][$id],
                'size'     => &$_FILES['my']['size'][$id],
            );
            
            $upload = new Base_Upload($file);
            $upload->setName(Base_String::getUnique(10));
            $upload->setExt('jpg');
            $upload->setMaxSize('2M');
            // пока реализована проверка по mime-типу 
            // подразумевается, что проверка для изображений, например, будет производиться в класе-наследнике 
            $upload->setAllowableType('image/jpeg');

            if ($upload->getErrors())
            {
                print_r($upload->getErrors());
            }
            else
            {
                $upload->copy(DOCUMENT_ROOT.DIRECTORY_SEPARATOR.'tmp');
            }
        }
    }
}
 

korpus

злой бобёр
Re: правильно ли я подхожу к написанию Uploader-a

Автор оригинала: triumvirat
PHP:
            $upload->setAllowableType('image/jpeg');
Возможно, все допустимые параметры загружаемого файла следует указывать в конструкторе. А так эту строчку можно будет забыть, и тогда будет загружен некорректный файл, что плохо с точки зрения безопасности.

-~{}~ 08.09.10 17:46:

Надо сделать эту функцию protected и вызывать из конструктора.
 

dimagolov

Новичок
korpus, а ты не допускаешь возможности загрузки произвольных файлов?
 

AmdY

Пью пиво
Команда форума
ИМХО, лучше так
PHP:
if ($this->getRequest()->isPost()) //нафик статику
{
    $upload = $this->getUploader('my'); // нафик new Класс
    if ($upload->hasFiles()) // нафик $_FILES
    {
            // задаём маску {NAME} - реальное имя {EXT} - расширение
            $upload->setName("upload/{UNIQ}.{EXT}");
            // можно {I} - счётчик, если фалов несколько
            //$upload->setName("upload/{UNIQ}_{I}.jpg"); // или так
            $upload->setMaxSize('2M');
            $upload->setAllowableType('image/jpeg');
            if (!$upload->load()) {
                print_r($upload->getErrors());
            }
        }
    }
}
-~{}~ 08.09.10 17:31:

если напишешь так, то брось мне исходники, а то тоже такой класс нужен, я сделал пока затычку :(
 

korpus

злой бобёр
Автор оригинала: dimagolov
korpus, а ты не допускаешь возможности загрузки произвольных файлов?
Тогда в этих protected методах должно быть предусмотрено поведение по умолчанию, когда разрешены любые файлы. Правда, если в конструктор передавать эти данные, вызов этого метода очень увеличится в размерах и станет сложным для восприятия. Как вариант - передавать массив допускаемых параметров.
$permission=array('type'='image/jpeg', 'size'=>200) и т.д.

-~{}~ 08.09.10 18:59:

AmdY, кстати верно, что лучше всю работу делать внутри класса и избегать ненужных действий снаружи. Нафиг new и $_FILES
 

Духовность™

Продвинутый новичок
$upload = $this->getUploader('my'); // нафик new Класс
вы забыли, что в FILES может быть несколько файлов? Как вы их тогда собираетесь обрабатывать поштучно?

AmdY
Интересные идеи. Но
$upload->setName("upload/{UNIQ}.{EXT}");
мне не катит, ибо в БД мне надо сохранять имя файла, расширение, размер и т.д. Поэтому придется все же передавать частями.
 

HraKK

Мудак
Команда форума
triumvirat
мне не катит, ибо в БД мне надо сохранять имя файла, расширение, размер и т.д. Поэтому придется все же передавать частями.
А ты сделай такой класс, который наследуя и подменяя одну функцию получишь то что хочет AmdY. И напиши этот расширенный класс для Амди.

З.Ы. У меня бы этот класс занял где-то бы 6-8 файлов)
 

AmdY

Пью пиво
Команда форума
Автор оригинала: triumvirat
вы забыли, что в FILES может быть несколько файлов? Как вы их тогда собираетесь обрабатывать поштучно?
здесь две ситуации - файлы с разными именами my0 и my1 , предпологается что и правила у них могут быть разные
PHP:
$upload0 = $this->getUploader('my');
$upload1 = $this->getUploader('my2');
файлы в неймспейсе form[my][]
PHP:
$upload = $this->getUploader('my');
$upload->setName("upload/{UNIQ}_{I}.{EXT}"); // используем счётчик 
$data = $upload->load();
Интересные идеи. Но
мне не катит, ибо в БД мне надо сохранять имя файла, расширение, размер и т.д. Поэтому придется все же передавать частями.
в $data соответственно возвращаются данные сгенерированных по маске, можно и всё остальное размер, название превьюх и т.д.

З.Ы. У меня бы этот класс занял где-то бы 6-8 файлов)
у меня костыль размером один метод :(, но придётся ещё допиливать методы для обработчиков разных типов
 

HraKK

Мудак
Команда форума
зачем? и что бы там было? абстрактные классы и интерфейсы?
В том числе они. Но я еще бы разбил на подсущьности) Зато потом никаких напильников не понадобится.
 

Wicked

Новичок
мне вот инетерсно, а реальна ли ситуация, что будет загружена картинка, переименованная в *.php и ее какой-нибудь такой аплоадер, основанный на mime magic, сочтёт безопасной?
 

Активист

Активист
Команда форума
> Нафиг new и $_FILES

> $upload = $this->getUploader('my'); // нафик new Класс
> if ($upload->hasFiles()) // нафик $_FILES

Вот я писал давно GUI, еще когда была Win 3.1 и была бета Win 95.

Вот интересно, чем плох new? Зачем сувать все в непонятные методы getUploader('my'),причем, находящейся в том же классе. Это не ООП, это хрень какая-то получается, в чем смысл его? Проще инициализировать новый экземплях класса, а не вызывать некий обработчик, который делает ненужные вещи, а все это не только - непонятность, выражаемая в логике, и лишнее процессорное время.

Все прочитали паттерны проектирования и забыли прочитать антипаттерны проектирования, имхо.

Вот мы имеем объект - квадратная кнопка (привычная нам в винде), который имеет ряд свойств и методов (н-р, координаты, цвет бордера, фон, надпись, формат текста), в контексте ООП, квадрат - это объект, имеет н-р следующие свойства - координаты левой верхней вершины X1, Y1), длина и ширина кнопки, ее фон, к этому классу прикручены методы, н-р, метод, отвечающий за нажатие кнопки, и т.п.. Так как этих объектов может быть много, то там нах не нужен обработчик типа getBtn('id'), ибо абсурдно, а проще вызвать new btn(x1,y1, bg, color, uniqueid);

Далее сам объект btn использует другие классы, инициализируя объекты или выполняя статические свойства, например, объект, отвечающий за нанесение текста в центр кнопки, объект отвечающий за нанесение прямых линий и т.п.

При выполнении например, нажатия на кнопку, выполняется метод, н-р, btnDown, который меняет цвета бордеров, коих четыре.

Объект text, который наносит изображения, может имеет свой ряд методов и классов, задействованы, кстати. и обработчки файлов фонтов (я в свое время сам рисовал фонты, типа Italic, Bold, занятие конечно интересное).

Ну так вот, есть некий обработчик, контролер, который получая прерывание от драйвера мыши (IRQ), передает управление модели, которая определяет, что произошло (н-р нажата левая кнопка мышип), где же сейчас координаты мыши, и запускает нужные обработчик в View (вызывается методот down нужного объекта, который уже и работает с объектом btn c uniqueid), далее, view говорит model - ок, я отрисовал, и далее модель делает что-то, что должно было вызвано по событию down именно этой кнопки.

Если говорить, о наследовании - то оно примеримо к тому, что , например, вместо квадратной кнопки, нужно нарисовать полукруглую кнопку (с закругленными краями), вот только тогда применяется наследование, где меняется, например, метод drow, а остальное остается неизменным.

Прошу отметить, что подобные задачи реально реализовать даже не применяя ООП в таких языках как ASM и в даже в школьном quick basic (сам видел, и как-то сделал), правда там, уже все будет практически линейное (ASM) либо на процедурах и функциях и неймспейсах (qbasic).

Т.е., это и есть ООП, программирование, это и есть применение MVC, которое пришло к нам как раз и такого рода задач (GUI), неумелое применение (в случае с непонятными $this->getUploader('my')) и излишнее дроблении на классы, подклассы сущности и подсущности, суваенимем пустых интерфесов, ненужных наследований и т.п., производит совсем другое впечатление от просмотра подобного кода, где понятно лишь автору, потому что у него такое "особенное видение" MVC (например, код HraKK у меня вообще вызывает взрыв в мозге от такого количества избыточного кода и файлов), которое кстати, крайне неумело используется в скриптовом языке и всунуто туда через напильник, но зато это типа модно. Поскольку сама по себе технология WEB не подразумевает MVC в своем изначальном понимании, а является лишь функцией, с входными и выходными данными (т.е., получил от юзера данные, отдал обработанные) - по своей сути это - типичная функция.

В контексте этой задачи, Uploader - это новый, уникальныйобъект, в который должны быть переданы следующие свойства, информация о файле (то что лежит в $_FILES), и конечное директория, куда файл будет залит ну и что он заливает (тип файла или документа), умные скажут, что сам uploader должен обращаться к некому классу валидатору загрузчика, но это мягко говоря глупо, поскольку валидатор (разрешена ли загрузка именного этого файла) вынесенный в "подсущность (отдельный класс)", ему это не нужно - он не будет "размножен" на объекты (много валидаров), а будет применяться лишь в классе uploader'а. ДА и ВОООБЩЕ, применение для этого класса делает лишь вид, поскольку по своей сути ТАКЖЕ ЯВЛЯЕТСЯ ЛИШЬ функцией - взял данные , обработал, скопировал, вернул реальзультат), ЭТО ДАЖЕ НЕ ОБЪЕКТ, сувать ли это в класс, дело каждого (можно просто сделать функцию), полезно лишь будет для общего вида (типа я юзаю ООП) ну или полезно для общей удобочитаемости (типизации кода)

Поскольку объект uploader'a должен быть уникальным и никаких обрабтчиков его вызова не нужны по определению задачей, то, он тупо вызвается через new obj, причем, основные параметры можно (и даже нужно) засунуть в конструктор, и не вызывать методы установки свойств setName, setMaxSize('2M'), setAllowableType('image/jpeg'), поскольку эти методы не будут вызваны повторно на всем участке кода, а указаны ЛИШЬ ОДНАЖДЫ, при инициализации объекта.

-~{}~ 09.09.10 14:00:

Да, и зачем обрабатывать $_FILES каким-то мега-мега объектом (методом объекта), если $_FILES уже обработан, подан и даже посолен?

-~{}~ 09.09.10 14:08:

Да, хочется отметить (наболело от просмотра кода некоторых) применение __set и __get, то, __set например используется для того, что бы когда изменено свойство объекта, н-р изменилась координата x объекта font (нанесение текста на кнопке), объекта btn, стереть отрисованной ранее текст (например вызвать метот erase, изменить координату x и вызвать повторно метод print, что, например, очень удобно при организации движения объекта.

-~{}~ 09.09.10 14:24:

Вообще, ООП можно понять, только тогда, когда будет написано ХОТЬ ОДНО нормальное приложение, а не скрипт, который отрабатывает милисикунды, и его работа ни как не изменяется в процессе выполнения, ооп в скриптовых языках - это костыль :)
 

whirlwind

TDD infected, paranoid
Активист а теперь то же самое, тока одной фразой можешь? :D
 

Духовность™

Продвинутый новичок
Активист
круто! во Многом согласен!!!

-~{}~ 09.09.10 11:30:

мне вот инетерсно, а реальна ли ситуация, что будет загружена картинка, переименованная в *.php и ее какой-нибудь такой аплоадер, основанный на mime magic, сочтёт безопасной?
Как это реально? Не реально, ибо браузер посылает mie основываясь на расширении. Это защита от уж совсем неумелого кулхацкера.
 

AmdY

Пью пиво
Команда форума
Активист
согласен, согласен, согласен. но когда в контроллере все доступные сервисы можно получить через get<Сервис>() - это очень удобно и не нужно долго вспоминать как называется класс, не нужно читать тонны доков.
Когда этих сервисов много, то ясное дело на каждый не нагородишься геттеров.
 
Сверху