doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 504 forks source link

Using Builders without class after 2.0 #2102

Open Steveb-p opened 5 years ago

Steveb-p commented 5 years ago
Q A
Version 2.0.2 after migrating from 1.3

Support Question

Query and Aggregation builders in version before 2.0 were able to work with queries against collections that did not have their own class.

I used to be able to instantiate them and then pass a string indicating what collection I wanted to ask. Then if the string I passed was not a valid class name, a direct collection name was assumed.

Was this feature never intended to work? Was it dropped intentionally, or rather it was a case of a feature that was cut off unintentionally?

From what I've seen in the code, both Builders immediately ask for ClassMetadata instance, which is the reason why passing collection name no longer works. I've managed to get around the issue by injecting a listener for onClassMetadataNotFound event, creating a fake class for it to work with and adjusting collection name in Event instance accordingly. However I feel it's a rather hacky solution.

My use-case is a "legacy" application that partitions data between multiple collections based another object's property.

Depending on whether it was intentional or not, I would like to propose:

  1. Expand upgrade document to add information that it is no longer possible to query collections directly.
  2. If this solution has no side-effects that I didn't notice, then maybe add it to the docs as a workaround.
  3. Allow getClassMetadata to fail silently in both builders and/or allow them to work without ClassMetadata instance in those cases.

WDYT?

malarzm commented 5 years ago

I think it was unintentional feature drop however I had no clue such thing was possible with 1.x :D

malarzm commented 5 years ago

But I believe such thing was possible due to all builders in 1.x being inherited from doctrine/mongodb which was dealing with plain collections. FWIW I think it would be nice to preserve this functionality if that's not too much hassle on our side.

Steveb-p commented 5 years ago

But I believe such thing was possible due to all builders in 1.x being inherited from doctrine/mongodb which was dealing with plain collections.

Yes, that's the case actually. Since in 2.0 they no longer extend from those base classes, hence no ability of building those queries with support of ODM (without anythying that relied on metadata, of course).

Using onClassMetadataNotFound seems to have no obvious side-effects - MetadataFactory caches entries using original requested class name, so asking for another collection afterwards seem to work.

alcaeus commented 5 years ago

I'll have to take a look. That definitely wasn't an intended BC break, and if we can easily bring that back, I'd be happy to do so.

@Steveb-p do you want to cook up some test cases or even give the implementation a shot?

Steveb-p commented 5 years ago

do you want to cook up some test cases or even give the implementation a shot?

Yes, I'll try to work it out this week.

stale[bot] commented 4 years ago

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.

malarzm commented 4 years ago

Bad bot!

alcaeus commented 4 years ago

@malarzm putting the issue into a project (e.g. the Roadmap) keeps the bot away :)

Steveb-p commented 4 years ago

I like the stale bot though. It exists as a constant reminder of my failure to deliver promises and things I forgot about :smiley_cat:

alcaeus commented 4 years ago

@Steveb-p I'm glad you like it - I always appreciate it reminding me about issues and PRs that I forgot to follow up on :)