mongodb-odm icon indicating copy to clipboard operation
mongodb-odm copied to clipboard

PersistentCollection doesn't have the matching method

Open dorongutman opened this issue 10 years ago • 11 comments

Both https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/PersistentCollection.php#L861 and https://github.com/doctrine/collections/blob/master/lib/Doctrine/Common/Collections/ArrayCollection.php#L358 (which is the type one needs to use on a ReferenceMany field) have a "matching" method for searching (via a Criteria) a set of items in the collections.

However - https://github.com/doctrine/mongodb-odm/blob/master/lib/Doctrine/ODM/MongoDB/PersistentCollection.php (which is what returned on a ReferenceMany field) doesn't have that method.

How can I search in the ReferenceMany field ?

dorongutman avatar Aug 24 '14 09:08 dorongutman

This feature isn't yet implemented. Quoting the 1.0 changelog:

The base DocumentRepository class now implements the Selectable interface from the Criteria API in Doctrine Collections 1.1. This brings some consistency with ORM; however, ODM's PersistentCollection class does not yet support Selectable. This functionality was introduced in #699.

jmikola avatar Aug 26 '14 20:08 jmikola

Hi guys, any update on this issue?

youuri avatar Dec 06 '16 08:12 youuri

None besides what's written above. It's in the 1.x milestone, meaning we are willing to add it to one of the next 1.x releases, but nothing more than that. You could always help get things started by creating a pull request!

alcaeus avatar Dec 07 '16 06:12 alcaeus

I thought the custom collections feature made this less important?

On 7 Dec 2016 5:23 pm, "Andreas" [email protected] wrote:

None besides what's written above. It's in the 1.x milestone, meaning we are willing to add it to one of the next 1.x releases, but nothing more than that. You could always help get things started by creating a pull request!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/doctrine/mongodb-odm/issues/929#issuecomment-265366258, or mute the thread https://github.com/notifications/unsubscribe-auth/AAu3mPs1jYpa0mXYL-04fi0AZZzgHvd_ks5rFlDRgaJpZM4Cahuy .

redthor avatar Dec 07 '16 06:12 redthor

+1 for having this feature

I checked the ArrayCollection::matching() implementation, it can be implemented in a similar way in PersistentCollection I believe. Ideally using PersistentCollection's methods instead of array_*. But then few more methods needs to be implemented, i.e. PersistentCollection::reverse.

What do you think?

@alcaeus

svscorp avatar Jan 03 '17 00:01 svscorp

@svscorp I haven't taken a look in a while. Since you have an idea about how to implement it, could you create a pull request?

alcaeus avatar Jan 04 '17 09:01 alcaeus

I took a bit of a look at this yesterday night (about how it was implemented in ORM too). I'm a bit lost as I never really looked into the code before, but here what I saw for now, let me know if I'm completely wrong.

In ORM's matching method (in PersistentCollection) it call a persister->loadCriteria(Criteria) to query the referenced Entities. The thing is, in ORM, there is a specific persisters for Relations (I remember a persister for Many relationship at least) to implement the loadCriteria differently while in ODM we only have a DocumentPersister.

I didn't take a proper look at how the persister is choosen yet.

I guess the best to do would the same and query the referenced Document (for ReferenceOne and ReferenceMany at least, as Embed Documents will be available without querying them when the "main Document" is fetched from DB), rather than query them all and then filter them like in an array ?

About the multiple persisters in ORM, Is there a need to add multiple persisters in ODM to differentiate the relations ? If yes, how to determine which persister to instantiate ?

Do we have to change the mapping array (in persistentCollection) into a Class like in ORM ? (In ORM the mapping equivalent is called association)

Do we have to add new Collection class (like the LazyCriteriaCollection and others in ORM that doesn't seem to have equivalent in ODM) ?

I still need to look a LOT of things; how (and where) the query to referenced Documents is made, to be able to add the Criteria to it ans surely other things.

ps : Not related but there is a lot of methods with missing docBlock (return statement, param etc.), is that intentional ?

ps2 : I wrote that quickly during a break, I maybe forgot some things or it may be a bit confusing.

etshy avatar Apr 10 '19 11:04 etshy

The implementation differs a lot from ORM. For one, we have a lot more flavours of associations than ORM. For example, for any owning-side collection, matching wouldn't really be a performance boost (be it references or embedded relationships): since the collection only keeps a list of documents (either raw embedded document data or references) that should be in it, we can't apply a custom query to the collection. So those will always have to be walked like any other generic ArrayCollection.

However, if we're dealing with a inverse reference-many that isn't loaded by a repositoryMethod (which also doesn't exist in ORM), we can return a new PersistentCollection instance that modifies the criteria property from the mapping, adding whatever criteria was received in the call to matching().

I've previously hacked this together but haven't found the time to polish it for inclusion in 2.0 - I can gladly hack it back in and push a WIP branch for others to pick up on.

As for your question regarding a new Collection class, in our case we duplicated it, but that was because we couldn't change the original implementation. For ODM, we'd have to change PersistentCollectionInterface to also extend Selectable, which is a BC break and won't happen in 2.0. Alternatively, this can be moved to PersistentCollection entirely. This is one of the reasons why I've shied away from implementing this in 2.0: it adds a lot of complexity that we're trying to avoid this late in 2.0 development (I'd like to get it done rather soon). I believe it's best to sit on this until 2.1 and 2.2 and then add this in a backward compatible fashion.

alcaeus avatar Apr 11 '19 05:04 alcaeus

Hey. I was in break for the past 2 weeks, and I couldn't really continue working on this.

If it's for 2.1 (or more) I guess it's not really urgent, then.

I think I'll take a better look of the sources, to know how it works, for now.

Once I know better about the sources, how it works etc, I could really help.

(I'll have to take a really good look at the 2.x sources, as I only worked with 1.x until now)

etshy avatar May 03 '19 08:05 etshy

2.x is not very different except for coding standards and the low-level logic. Collections are very similar. As this feature won’t make it into 2.0 either way, there’s no need to hurry.

alcaeus avatar May 03 '19 14:05 alcaeus

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed in a week if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 06 '21 20:09 stale[bot]