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

RemoveElement on sorted collection #623

Closed
dan19 opened this issue Jun 21, 2013 · 5 comments
Closed

RemoveElement on sorted collection #623

dan19 opened this issue Jun 21, 2013 · 5 comments
Assignees
Milestone

Comments

@dan19
Copy link

dan19 commented Jun 21, 2013

I have a document Whatever with a collection "users" which reference many User.
This collection is sorted with an annotation.

/**
* @ODM\ReferenceMany(targetDocument="User", simple=true, sort={"lastname"="asc", "firstname"="asc"})
*/
protected $users = array();

If I try to delete one user reference from the collection.

$whatever->getUsers()->removeElement($user);

The user deleted is not the one I want to delete. There is an issue with the key here because of the sort annotation. The removeElement method should remove the element at the key x based on the order in the database.

Log :
doctrine.INFO: MongoDB query: {"update":true,"query":{"_id":{"$id":"51c4a13ffa4634362a000061"}},"newObj":{"$unset":{"users.2":true}},"options":{"safe":true,"fsync":false,"timeout":30000},"db":"something","collection":"Whatever"}

@jmikola
Copy link
Member

jmikola commented Jun 26, 2013

This has come up on another issue/PR, but I can't seem to find it at the moment. Sort should really only be used on the inverse side of a ReferenceMany relationship, and in those cases changes to the PersistentCollection itself (not the documents it contains) would be ignored, since there really is no array on the parent document.

The problem with sorting in your case is that once the PersistentCollection is initialized, its ordering may immediately differ from what's actually stored in the database.

@j: Do you recall the other issue I was thinking of? It might have been one that you were involved with.

@Jimgitsit
Copy link

Am am also experiencing this issue. Has there been any resolution to this?

@malarzm
Copy link
Member

malarzm commented Jul 1, 2015

@Jimgitsit I believe most important part is @jmikola's

Sort should really only be used on the inverse side of a ReferenceMany relationship

and maybe we should throw exception if owning side tries to use this combination? If you really want to stick with sort here try using setArray or atomicSetArray for your collection, it should do the trick. I think I've seen a PR that would fix removing sorted references using addToSet or pushAll strategies but I need to find it (that wasn't sole purpose of that PR but it will be enough for us)

@malarzm malarzm added this to the 1.0.0 milestone Jul 1, 2015
@malarzm malarzm self-assigned this Jul 30, 2015
@malarzm
Copy link
Member

malarzm commented Aug 2, 2015

For the record PR I had in mind is #524

@malarzm
Copy link
Member

malarzm commented Aug 13, 2015

#1191 has been merged manually in 56796fa

@malarzm malarzm closed this as completed Aug 13, 2015
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

4 participants