fuelphp / upload

FuelPHP Framework - File upload library
MIT License
26 stars 16 forks source link

Upload::save($index) problems #8

Closed huglester closed 9 years ago

huglester commented 10 years ago

Hello,

I have a little problem using Upload class in some PyroCMS project.

I have basically copied the Facade class from fuel/core/upload.php ;)

With code like this:

            $uploader = new Upload($config);
            $uploader->process();

            // if there are any valid files
            if (Upload::is_valid())
            {
                // save them according to the config
                Upload::save();

                // call a model method to update the database
                // Model_Uploads::add(Upload::get_files());
            }

            dd(Upload::get_files());

I get good results, files uploaded, like on the pic: http://pic.webas.lt/HWw7B4U3nx.png (array have 12 items)

But when using code like this:

            $uploader = new Upload($config);
            $uploader->process();

            if ($uploader->is_valid())
            {
                // get the list of successfully uploaded files
                foreach($uploader->get_files('userfile') as $k => $file)
                {
                    $uploader->save($k);
                    d($file);
                    // $item->image = $file['saved_as'];
                    // break; // we allow only one upload of this type
                }

                $count = 0;
                foreach($uploader->get_files('workexample') as $k => $file)
                {
                    ++$count;
                    $uploader->save($k);

                    // CvbankPhotos_m::addFiles($this->sentry->getUser()->id, $file);
                    if ($count === 5)
                    {
                        break;
                    }
                }
            }

I get results like this: http://pic.webas.lt/hfN1lvOAQQ.png - files ARE uploaded... but as you can see, array have 11 keys (not 12). there is no 'saved_to' key, and 'saved_as' key is null...

SInce I basicly copied the Fuel 1.8 facade class... I think it is related to fuelphp/upload itself...

Thank you Sir!

WanWizard commented 10 years ago

This is really a Fuel v1 question, so it should be reported in the fuel/core repo issue tracker.

What happens if you do:

foreach ($uploader::instance()->getValidFiles() as $file)
{
    dd($file); // dumps the File object (I assume is what dd() does)
    d($file->save());
    dd((array) $file); // dumps the internal data structure as an array
}
huglester commented 10 years ago

thought it's 2.x :)

so the results for those dd() methods are here: http://bin.fuelphp.com/snippet/view/zY

$file->save() - returns true

WanWizard commented 10 years ago

I don't see anything immediately wrong.

You are sure you have the correct version of fuelphp/upload? Looking at the code of the save() method, the only way to get out of it without 'filename' being set on the container, and without isValid being set to false, is if an exception is thrown.

huglester commented 10 years ago

I probably don't get something... because:

'filename' => NULL

so probably something is wrong? as filename is not passed or something...

Well, I will try to find a way around this, to get saved_as and saved_to. since I want to make some validation, which is based on file count and name of the upload field.

Well the version is the master one, last command i see in git log is:

WanWizard commented 10 years ago

The dump indicates validation was called, and the input validated.

It simply looks like save() isn't called on the object. As it checks the path (which bails out with an error if it doesn't exist), and at the next step it will already determine the filename (at L#343).

You have have to do some debugging, because I'm clueless atm.

huglester commented 10 years ago

Can't find the bug in here, or in my setup.. I started to work directly with Fuel\Upload\Upload:

            $config = array(
                'path' => FCPATH.'uploads/fuel/',
                'change_case' => 'lower',
                'normalize' => true,
                'auto_process' => true,
                'normalize_separator' => '-',
                'ext_whitelist' => array('img', 'jpg', 'jpeg', 'gif', 'png'),
            );

            $uploader = new Fuel\Upload\Upload($config);

            foreach($uploader->getValidFiles() as $k => $file)
            {
                $file->save();
                d($file);
            }

but the $container still does not have saved_to and saved_as, tried to debug Upload class, but as for now no luck...

the container has:

array(11) {
    'element' =>
    string(8) "userfile"
    'filename' =>
    string(8) "s1_9.png"
    'name' =>
    string(6) "s1.png"
    'type' =>
    string(9) "image/png"
    'tmp_name' =>
    string(14) "/tmp/phpjJ5FhR"
    'error' =>
    int(0)
    'size' =>
    int(22311)
    'extension' =>
    string(3) "png"
    'basename' =>
    string(2) "s1"
    'mimetype' =>
    string(9) "image/png"
    'path' =>
    string(45) "/var/www/vhosts/welldone/public/uploads/fuel/"
  }

I also tried to do: $uploader->getValidFiles('DOES_NOT_EXIST'); And get: ErrorException [ Fatal Error ]: Call to a member function isValid() on a non-object (FCPATH/vendor/fuelphp/upload/src/Fuel/Upload/Upload.php [ 276 ])

Think this is invalid behaviour

WanWizard commented 10 years ago

"saved_to" and "saved_as" are v1.x values, that the v2 class doesn't have or use, it uses the properties of the container of the File object. The v1 facade class (Upload) maps those for backward compatibility.

And yes, that is a bug, it doesn't validate if whay you put in exists, I'll fix that