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

Inject EntityManager into onCreate hook #5201

Closed
Kalos-S opened this issue Feb 2, 2024 · 2 comments
Closed

Inject EntityManager into onCreate hook #5201

Kalos-S opened this issue Feb 2, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@Kalos-S
Copy link

Kalos-S commented Feb 2, 2024

Is your feature request related to a problem? Please describe.
When implementing multi-tenancy there is no (clear) way to automatically populate a tenant_id column from a filter previously set on the EntityManager during initialization.

Describe the solution you'd like
Pass the current EntityManager instance to onCreate calls, perhaps as a second argument.

Describe alternatives you've considered
Using the RequestContext class to get the current EntityManager in the onCreate call is possible. This is a very fragile solution and can lead to a lot of potential edge cases if the AsyncLocalStorage is cleaned up before execution of the hook or RequestContext class references differ though.

@Kalos-S Kalos-S added the enhancement New feature or request label Feb 2, 2024
@B4nan
Copy link
Member

B4nan commented Feb 2, 2024

Using the RequestContext class to get the current EntityManager in the onCreate call is possible. This is a very fragile solution and can lead to a lot of potential edge cases if the AsyncLocalStorage is cleaned up before execution of the hook or RequestContext class references differ though.

This feels like you are afraid of problems that are not there, the ALS solution seems to be pretty stable to me, you won't get this executed outside of the context, as it gets executed only during flush.

Another more explicit alternative is beforeCreate hook where you already get access to the current EM.

I don't mind doing this, but its not like "there is no clear way", or that the solution you already have would be bad.

@Kalos-S
Copy link
Author

Kalos-S commented Feb 2, 2024

Thanks for your quick reply! I've personally run into issues with context loss using the ALS solution, which is why I'm suggesting an alternative in the first place. While use of the beforeCreate hook seems promising, I believe directly passing the EntityManager to onCreate could make my current implementation simpler and more robust.

@B4nan B4nan closed this as completed in a964aeb Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants