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

Using Builders without class after 2.0 #2102

Open
Steveb-p opened this issue Nov 9, 2019 · 10 comments
Open

Using Builders without class after 2.0 #2102

Steveb-p opened this issue Nov 9, 2019 · 10 comments

Comments

@Steveb-p
Copy link
Contributor

Steveb-p commented Nov 9, 2019

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
Copy link
Member

malarzm commented Nov 9, 2019

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

@malarzm
Copy link
Member

malarzm commented Nov 9, 2019

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
Copy link
Contributor Author

Steveb-p commented Nov 9, 2019

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
Copy link
Member

alcaeus commented Nov 11, 2019

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
Copy link
Contributor Author

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
Copy link

stale bot commented Dec 11, 2019

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 stale bot added the Stale Issues that have not seen any activity in 30 days label Dec 11, 2019
@malarzm malarzm removed the Stale Issues that have not seen any activity in 30 days label Dec 11, 2019
@malarzm
Copy link
Member

malarzm commented Dec 11, 2019

Bad bot!

@alcaeus
Copy link
Member

alcaeus commented Dec 12, 2019

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

@Steveb-p
Copy link
Contributor Author

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

@alcaeus
Copy link
Member

alcaeus commented Dec 12, 2019

@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 :)

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

No branches or pull requests

3 participants