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

The file is not updated if there are not other changes in the entity #8

Closed
vladcosorg opened this issue Feb 2, 2012 · 28 comments · May be fixed by #617
Closed

The file is not updated if there are not other changes in the entity #8

vladcosorg opened this issue Feb 2, 2012 · 28 comments · May be fixed by #617

Comments

@vladcosorg
Copy link

Say if i have a already existing record, and i'm trying to update it, in case if i ONLY upload file and no other entity properties get changed, the preUpdate event is not called, so the file is not uploaded.

Currently the workaround is to introduce some changes into the entity, before saving it.

@dustin10
Copy link
Owner

dustin10 commented Feb 2, 2012

I have come across this issue in my current project, but I have not had time to dive into it further. I have just implemented a little hack in my entities right now similar to the following:

public function setFile($file)
{
    $this->file = $file;

    if ($file instanceof UploadedFile) {
        $this->setUpdatedAt(new \DateTime());
    }
}

Hopefully I will have time to look into this soon.

@vladcosorg
Copy link
Author

Thanks :)

@Blackskyliner
Copy link

Uhh Ohh dirty! :)
But thanks ;)

@Problematic
Copy link

I believe this is an issue with the framework itself, and not this bundle in particular. I've run up against it as well, independently of this uploader, and I'm looking into it too.

@bnlab
Copy link

bnlab commented Mar 30, 2012

+1 ;)

@inmarelibero
Copy link
Contributor

cool workaround, hoping it will be fixed soon

@danielsan80
Copy link

thanks guys! You have saved my day!

@fkrauthan
Copy link

I have this issue too maybe this could be fixed in this bundle.

@thomaskonrad
Copy link
Contributor

Same issue here. I guess this is because Doctrine doesn't detect any changes when only a file is uploaded as the "file" member variable is not mapped as a database column.

I'm just guessing, though.

@MisterGlass
Copy link

This is an issue with doctrine, which wont persist the entity if it doesn't see any changes. I have an issue in the main Symfony project here: symfony/symfony#5150 (comment)

@Ocramius
Copy link

As I've explained to @aderuwe, this is kinda wrong.
This should not be pushed down to event listeners/subscribers attached to Doctrine ORM.
Since you're dealing with transitient fields here (not related with persistence) you are basically mixing persistence logic with the Bundle's logic. Don't do that.
Instead, I'd suggest to fix with something like

public function updateFile(UploadedFile $newFile, YourOwnServiceStuffYouRequire $service)

and perform all the persistence operations related to the file itself there.
The fix by @dustin10 is valid just to avoid breaking BC, but please stop doing this, and instead set a persistent field's value before flushing (Not in preFlush or other nasty hacks: really, just do it BEFORE interacting with Doctrine's UoW lifecycle!)

@ftassi
Copy link
Collaborator

ftassi commented Sep 2, 2012

Hi @Ocramius, I would like to discuss this with you, will you be available for a quick chat on this topic ?

@Ocramius
Copy link

Ocramius commented Sep 2, 2012

@ftassi ping me on IRC tomorrow eventually ( #doctrine )

@davidromani
Copy link
Contributor

Same problem, still waiting...

Thanks.

@stfalcon
Copy link
Contributor

stfalcon commented Nov 9, 2012

@dustin10, maybe you have a normal solution? much time has passed.

@Sydney-o9
Copy link

Doctrine uses identical operator (===) to compare changes between old and new values. The operator used on the same object with different data always return true. The best way is to clone the object.

@stfalcon
Copy link
Contributor

@stof, maybe you can help resolve this problem?

@stfalcon
Copy link
Contributor

Guys, I researched this issue and think it's unsolvable problem for current architecture :(

This hack should be added to the documentation

@Koc
Copy link
Contributor

Koc commented Nov 22, 2012

maybe preFlush event can helps?

@stfalcon
Copy link
Contributor

@Koc I tried. I couldn't get entity in preFlush event

@Baachi
Copy link
Contributor

Baachi commented Nov 22, 2012

The only way to go is, to use the vich_uploader.storage service manually.

$form = $this->createForm(new AcmeType(), $entity);
$form->bind($request);

if ($form->isValid()) {
    // Upload the new file
    $storage = $this->container->get('vich_uploader.storage');
    $storage->upload($entity);

   // Save the new filename
   $om->persist($entity);
   $om->flush();

    return new RidirectRespone('/');
}

@Ocramius Right?

@stfalcon
Copy link
Contributor

@Baachi maybe. but still doesn't work in SonataAdminBundle (from the box) :(

@Baachi
Copy link
Contributor

Baachi commented Nov 22, 2012

@stfalcon Hm i doesn't work with SonataAdminBundle.
But i use custom events to upload my images/files, so SonataAdminBundle should call his own events, too.

@MisterGlass
Copy link

Maybe you can bake the hack into the sonata code?

@stfalcon
Copy link
Contributor

@MisterGlass I thought about it. need to try

@ftassi
Copy link
Collaborator

ftassi commented Nov 24, 2012

Right now the bundle is coupled to doctrine's lifecycle callback, which means that with no changes to the entity, no callback are called and no upload is triggered.

With this coupling there are two options to solve this problem:

  1. Add an updated field as suggested here
  2. Manually upload the file as suggested here

The second one is my favorite and in the future I would like to loose coupling with persistence layer, but I've not implemented this yet and I'm afraid that I'll not do it soonish.

@ftassi ftassi closed this as completed Nov 24, 2012
@thomaskonrad
Copy link
Contributor

I think that if there is no generic fix for the moment then the workaround(s) should be added to the documentation.

@ftassi
Copy link
Collaborator

ftassi commented Nov 24, 2012

👍
We could add a "known issues" (or known limitations) section where we can explain this and why we can't use the id of the object for file name (ref: #14)

Any chance to work on a PR ?

gido added a commit to gido/VichUploaderBundle that referenced this issue Nov 18, 2013
ftassi pushed a commit that referenced this issue Nov 18, 2013
ftassi added a commit that referenced this issue Nov 18, 2013
Mondane added a commit to Mondane/VichUploaderBundle that referenced this issue Jun 30, 2015
Add 'instanceof'  check as written here: dustin10#8 (comment) to avoid this issue dustin10#297 (comment)
Koc added a commit to Koc/VichUploaderBundle that referenced this issue Sep 2, 2016
Koc added a commit to Koc/VichUploaderBundle that referenced this issue Sep 2, 2016
Koc added a commit to Koc/VichUploaderBundle that referenced this issue Sep 3, 2016
Koc added a commit to Koc/VichUploaderBundle that referenced this issue Sep 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.