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.

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

File duplication #5

Open
WanWizard opened this issue May 8, 2013 · 4 comments
Open

File duplication #5

WanWizard opened this issue May 8, 2013 · 4 comments

Comments

@WanWizard
Copy link
Member

When you run process() multiple times, duplicate File objects are created.

See fuel/core#1426

@itsa-sh
Copy link

itsa-sh commented May 17, 2013

I've ran the Upload.php class through vimdiff and discovered the following:

Autoloader will execute the _init method (which uses the default config, and therefore process the uploads). The processing is also executed a second time during the constructor (which, is created inside the _init) so custom configuration is ignored.

Example:

\Upload::process(\Config::get('some.config.upload')); will not exist, the autoloader executes the following:

https://github.com/fuel/core/blob/1.6/master/classes/upload.php#L93

Before the static::$upload->processFiles(); the constructor, using the same configuration (default) executes:

https://github.com/fuelphp/upload/blob/develop/src/FuelPHP/Upload/Upload.php#L99-L106

After some time looking at what has changed, my solution was to remove the following lines; however the auto_process should then run validate alone since the processing has already been done.

https://github.com/fuelphp/upload/blob/develop/src/FuelPHP/Upload/Upload.php#L101-L103

@WanWizard
Copy link
Member Author

You're commenting to a Fuel 2.0 repository with reference to a 1.6 file, which is not relevant.

This issue is that every time you call processFiles(), it does that. So calling it twice processes them twice, with duplicate file entries as a result.

The solution is not hacking into the 2.0 code, the solution is to make sure your application logic is ok, that is: either process automatically (which uses the default config) or manually (with an optional custom config), but not both.

The fact that you could get away with it in 1.x is because of a bug in the old 1.x code which would overwrite existing entries. With the 2.0 library, you can no longer get away with it.

@itsa-sh
Copy link

itsa-sh commented May 17, 2013

You're right in the final point; however you did link a post from version 1.6/1.7 to this section - specifically relating to the duplicate files. Also why then do you have one version of upload for both 1.x and 2.x? Surely you should have two versions for both; or fix fuel 1.x.

@WanWizard
Copy link
Member Author

I know. As a reminder that it's always better to make something monkey-proof. ;)

2.0 modules will gradually find it's way into the 1.x codebase, so it slowly evolves into 2.0, at which point the old 1.x facade classes will because a compatibility package, allowing you to migrate existing applications with minimal effort. Log was the first, Upload followed, and probably DB and ORM will be next.

Unlike some other frameworks, we take business continuity (from a dev's point of view) very serious.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants