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

Extend DI documentation for classes with multiple interfaces #19280

Closed
1 task done
puschie286 opened this issue Mar 13, 2024 · 5 comments
Closed
1 task done

Extend DI documentation for classes with multiple interfaces #19280

puschie286 opened this issue Mar 13, 2024 · 5 comments

Comments

@puschie286
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

The documentation doesn't describe how exposing the interfaces for a class with multiple interfaces can change the instantiation behavior (for singleton and scoped lifetime)

Describe the solution you'd like

We have 2 behaviors:

  1. Single instance for every interface (exposing all interfaces but not the class itself)
  2. Shared instance between interfaces (exposing all interfaces and the class itself)

This should be made clear in the ExposeService Attribute section.

But the behavior itself gives you two limits:

  1. you cant expose the interfaces only with a shared instance
  2. you cant expose the implementation class with separate instances for each interface

( well you can always fall back to manually register those cases )

Additional context

This is somehow surprising: you might try to prevent the usage of the implementation class by only exposing the interfaces but result in having multiple singleton instances (one for each interface).

Maybe this is even classified as bug, because you change the behavior (that is defined by the Dependency attribute) by the way you use the ExposeServices attribute

@maliming
Copy link
Member

hi @puschie286

Can you share some code to explain this?

Thanks.

@puschie286
Copy link
Author

puschie286 commented Mar 13, 2024

sure:

interface IFirstSingle;
interface IFirstMulti;
interface ISecondSingle;
interface ISecondMulti;

[ExposeServices( typeof( SingleInstance ), typeof( IFirstSingle ), typeof( ISecondSingle ) )]
class SingleInstance : IFirstSingle, ISecondSingle, ISingletonDependency;

[ExposeServices( typeof( IFirstMulti ), typeof( ISecondMulti ) )]
class MultipleInstances : IFirstMulti, ISecondMulti, ISingletonDependency;

public class TestClass
{
	public void Test( IServiceProvider provider )
	{
		var firstSingle = provider.GetService<IFirstSingle>();
		var secondSingle = provider.GetService<ISecondSingle>();
		
		Debug.Assert( firstSingle == secondSingle );

		var firstMulti = provider.GetService<IFirstMulti>();
		var secondMulti = provider.GetService<ISecondMulti>();
		
		Debug.Assert( firstMulti != secondMulti );
	}
}

its just example code, but this is the behavior you will see when using ABP auto registration process

@maliming maliming self-assigned this Mar 14, 2024
@maliming maliming added this to the 8.2-preview milestone Mar 14, 2024
@maliming
Copy link
Member

hi

In this case, you only added 2 services(IFirstMulti and ISecondMulti) to DI. They cannot get a unique implementation because IFirstMulti and ISecondMulti are different.

you can always fall back to manually register those cases

You have to add a new service to the DI to manually register those cases

[ExposeServices( typeof( IFirstMulti ), typeof( ISecondMulti ) )]
class MultipleInstances : IFirstMulti, ISecondMulti, ISingletonDependency;

@maliming maliming removed their assignment Mar 14, 2024
@maliming maliming removed this from the 8.2-preview milestone Mar 14, 2024
@puschie286
Copy link
Author

Yes, i understand that - but its still somehow surprising that ExposeServices can change the behavior. Would be great if the documentation state something like "If you dont expose the class itself, each interface get their own instance"

@maliming
Copy link
Member

Thanks. I will mention this in the document.

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