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

Cannot register ProductCalculator because it is a Singleton #459

Open
ToxicKevinFerm opened this issue Dec 22, 2023 · 4 comments
Open

Cannot register ProductCalculator because it is a Singleton #459

ToxicKevinFerm opened this issue Dec 22, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@ToxicKevinFerm
Copy link

Describe the bug
I cannot register a custom ProductCalculator because it is a Singleton. I need to access HttpContext and Umbraco services outside of examine because my prices are decided by the currently logged in user.

Steps To Reproduce
Inject IHttpContextAccessor or MemberService in your ProductCalculator.

Expected behavior
I expect to be able to run ProductCalculator as scoped or at least not be locked to Singleton. When I use the built in AddScoped instead of AddUnique it doesn't pick up the service at all.

Umbraco Commerce version:
13.0.0

@ToxicKevinFerm ToxicKevinFerm added the bug Something isn't working label Dec 22, 2023
@mattbrailsford
Copy link
Contributor

Please provide more information of what is actually not possible in the singleton? HttpContextAccessor should be injectable in a singleton so it’s unclear in your steps to replicate what actually fails.

@ToxicKevinFerm
Copy link
Author

You're right, sorry. It's MemberManager that is not injectable. I need it to get a user from a ClaimsPrincipal as I'm using the Delivery API.

I have managed to fix it currently using IServiceScopeFactory, so it's not the end of the world, but it would be nice to be able to inject it normally.

@MichaelNielsenDK
Copy link

I had the exact same issue today with PriceAdjuster, where I had to calculate a discount based on members.

I solved it as @ToxicKevinFerm by using IServiceScopeFactory like this

using (var scope = _serviceProvider.CreateScope())
{
      var _memberManager = scope.ServiceProvider.GetRequiredService<IMemberManager>();
      var currentMember = await _memberManager.GetCurrentMemberAsync();
     // Rest of code here
}

But yea, being able to do regular DI would be preferable.

@mattbrailsford
Copy link
Contributor

I guess we could make the calculators transient as they shouldn't be storing any state, but I'd imagine this would be classed as a breaking change so I'll need to review a) if this has any side effects and b) when would make sense to change this.

I appreciate the feedback 👍

@mattbrailsford mattbrailsford added enhancement New feature or request and removed bug Something isn't working labels Feb 12, 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

3 participants