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

ManagerRegistry#getRepository() does not use getManagerForClass() internally #45

Closed
mnapoli opened this issue Mar 28, 2019 · 4 comments
Closed
Assignees
Labels
Bug Something isn't working
Milestone

Comments

@mnapoli
Copy link
Contributor

mnapoli commented Mar 28, 2019

Assuming the Foo class is not in the default entity manager:

  • works:
$objectManager = $managerRegistry->getManagerForClass(Foo::class);
$repository = $objectManager->getRepository(Foo::class);
  • works:
$repository = $managerRegistry->getRepository(Foo::class, <manager name>);
  • doesn't work (because Foo is looked into the default manager):
$repository = $managerRegistry->getRepository(Foo::class);

I would have thought the 3rd case would work: after all the manager registry can retrieve the correct manager using getManagerForClass() internally.


Would it make sense to have getRepository() use getManagerForClass() internally to retrieve the repository in the correct manager?

@Ocramius Ocramius changed the title getRepository does not use "getManagerForClass" internally ManagerRegistry#getRepository() does not use getManagerForClass() internally Mar 28, 2019
@alcaeus
Copy link
Member

alcaeus commented Apr 15, 2019

That is correct. After taking a look, I would suggest the following changes:

  • If a manager name was given as second argument, it will always attempt to get that manager
  • If no manager name was given, it will use getManagerForClass to get the first manager that handles this class
  • If the previous steps did not yield an object manager, the default manager of the registry will be used.

I have the changes made but still need to add a bunch of tests around this functionality since getRepository wasn't tested at all yet.

@alcaeus alcaeus self-assigned this Apr 15, 2019
@alcaeus alcaeus added the Enhancement New feature or request label Apr 15, 2019
@mnapoli
Copy link
Contributor Author

mnapoli commented Apr 15, 2019

oh awesome thank you @alcaeus!

@alcaeus
Copy link
Member

alcaeus commented Apr 23, 2019

This will be fixed in 1.1.1.

@alcaeus alcaeus closed this as completed Apr 23, 2019
@alcaeus alcaeus added Bug Something isn't working and removed Enhancement New feature or request labels Apr 23, 2019
@alcaeus alcaeus added this to the 1.1.1 milestone Apr 23, 2019
@mnapoli
Copy link
Contributor Author

mnapoli commented Apr 23, 2019

Thank you for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants