Add check in doUpdate() to make sure the document containing the file is returned #61

Merged
merged 1 commit into from Sep 9, 2012

Conversation

Projects
None yet
3 participants
@pgodel
Contributor

pgodel commented Jul 10, 2012

Added check in doUpdate() to make sure the document containing the file is returned when updating an existing document with a new file.

I had this rare issue when connecting to a replicaSet. Issue appears to be fixed in PHP mongo driver already but a check here would not hurt.

Added check in doUpdate() to make sure the document containing the fi…
…le is returned when updating an existing document with a new file.
@jmikola

This comment has been minimized.

Show comment Hide comment
@jmikola

jmikola Jul 13, 2012

Member

I don't understand why doUpdate() is using findAndRemove() at all. The MongoGridFS docs have an example of updating a file, but doUpdate() has this comment:

It is impossible to update a file on the grid so we have to remove it and persist a new file with the same data

Is this because it's not possible to update a file's chunk data without also destroying the metadata? So we use findAndRemove() to fetch the metadata?

I imagine the PECL driver wouldn't error out if you attempted to update something that didn't exist, so perhaps we should NOP if we don't find an existing file.

Member

jmikola commented Jul 13, 2012

I don't understand why doUpdate() is using findAndRemove() at all. The MongoGridFS docs have an example of updating a file, but doUpdate() has this comment:

It is impossible to update a file on the grid so we have to remove it and persist a new file with the same data

Is this because it's not possible to update a file's chunk data without also destroying the metadata? So we use findAndRemove() to fetch the metadata?

I imagine the PECL driver wouldn't error out if you attempted to update something that didn't exist, so perhaps we should NOP if we don't find an existing file.

jwage added a commit that referenced this pull request Sep 9, 2012

Merge pull request #61 from pgodel/patch-1
Add check in doUpdate() to make sure the document containing the file is returned

@jwage jwage merged commit de0c453 into doctrine:master Sep 9, 2012

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