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

Service Descriptor attribute is not working as expected #182

Open
krishnan1159 opened this issue Sep 23, 2022 · 6 comments
Open

Service Descriptor attribute is not working as expected #182

krishnan1159 opened this issue Sep 23, 2022 · 6 comments

Comments

@krishnan1159
Copy link

krishnan1159 commented Sep 23, 2022

Hi Team,

For the below class structure,

Vehicle -> Implements IVehicle interface
GoodVehicle -> Extends Vehicle and implements IGoodVehicle interface
BadVehicle -> Extends Vehicle and implements IBadVehicle interface

I expect the following registration in services container

Service: IVehicle Lifetime: ScopedInstance: Vehicle
Service: IGoodVehicle Lifetime: TransientInstance: GoodVehicle
Service: IBadVehicle Lifetime: SingletonInstance: BadVehicle

But the actual registration is

Service: IVehicle Lifetime: TransientInstance: GoodVehicle
Service: IGoodVehicle Lifetime: TransientInstance: GoodVehicle
Service: IBadVehicle Lifetime: SingletonInstance: BadVehicle

[Scrutor.ServiceDescriptor(typeof(IGoodVehicle), ServiceLifetime.Transient)]
  public class GoodVehicle : Vehicle, IGoodVehicle
  {
      protected override string vehicleName => "GoodVehicle";
  }

  [Scrutor.ServiceDescriptor(typeof(IBadVehicle), ServiceLifetime.Singleton)]
  public class BadVehicle : Vehicle, IBadVehicle
  {
      protected override string vehicleName => "BadVehicle";
  }

  [Scrutor.ServiceDescriptor(typeof(IVehicle), ServiceLifetime.Scoped)]
  public class Vehicle : IVehicle
  {
      protected virtual string vehicleName { get { return "Vehicle"; } }

      public void drive()
      {
          Console.WriteLine($"Driving vehicle with name {vehicleName}");
      }
  }

  public interface IVehicle
  {
      public void drive();
  }

  public interface IGoodVehicle { }

  public interface IBadVehicle { }

I register the services using following scan

services.Scan(scan => scan
              .FromAssemblyDependencies(Assembly.GetEntryAssembly())
              .AddClasses(classes => classes.WithAttribute(typeof(Scrutor.ServiceDescriptorAttribute)))
              .UsingAttributes());

Root cause:
In the AttributeSelector, when we get the customAttributes we use type.GetCustomAttributes<ServiceDescriptorAttribute>(). This will get the attributes from parent classes as well. There is another api ( type.GetCustomAttributes<ServiceDescriptorAttribute>(false)) where we suppress traversing the parent classes. We can use that avoid this issue.

I have made the changes tested and wrote unit tests as well. Can I raise a pull request for this issue? Whether there are any workarounds for this issue?

@khellang
Copy link
Owner

Hello @krishnan1159!

I just ran the following test without issues:

using Microsoft.Extensions.DependencyInjection;
using Xunit;

namespace Scrutor.Tests
{
    public partial class ScanningTests
    {
        [Fact]
        public void Blah()
        {
            Collection.Scan(scan => scan
                .FromAssemblyOf<ScanningTests>()
                .AddClasses(c => c.Where(t => t.Name.EndsWith("Vehicle")))
                .UsingAttributes());

            var provider = Collection.BuildServiceProvider();

            Assert.IsType<Vehicle>(provider.GetRequiredService<IVehicle>());
            Assert.IsType<GoodVehicle>(provider.GetRequiredService<IGoodVehicle>());
            Assert.IsType<BadVehicle>(provider.GetRequiredService<IBadVehicle>());
        }

        [ServiceDescriptor(typeof(IGoodVehicle), ServiceLifetime.Transient)]
        public class GoodVehicle : Vehicle, IGoodVehicle { }

        [ServiceDescriptor(typeof(IBadVehicle), ServiceLifetime.Singleton)]
        public class BadVehicle : Vehicle, IBadVehicle { }

        [ServiceDescriptor(typeof(IVehicle), ServiceLifetime.Scoped)]
        public class Vehicle : IVehicle { }

        public interface IVehicle { }

        public interface IGoodVehicle { }

        public interface IBadVehicle { }
    }
}

Is that not what you would expect?

@khellang
Copy link
Owner

It results in the following registrations, which matches what you'd expect, right?

ServiceType ImplementationType
  IGoodVehicle GoodVehicle
  IBadVehicle BadVehicle
  IVehicle GoodVehicle
  IVehicle BadVehicle
  IVehicle Vehicle

@krishnan1159
Copy link
Author

krishnan1159 commented Sep 23, 2022

@khellang IVehicle should be registered only with Vehicle implementation. I don't want that to be registered with GoodVehicle and BadVehicle implementations. In my original code, I had all with the singelton lifetime. Notsure, whether that is causing the issue? Also, when I tried to fetch instance for IVehicle, I got GoodVehicle in the original code.

What is the registration strategy used for your testcase? In your testcase, in what order concerte classes are parsed. I guess that also might make a difference here.

@krishnan1159
Copy link
Author

@khellang Is it right to get all custom attributes from the parent classes also if you have ServiceDescriptor attribute?

@krishnan1159
Copy link
Author

krishnan1159 commented Sep 23, 2022

@khellang I made a small typo when I am writing the code. Can you please check now? Instead of IVehicle earlier I used Vehicle

old block

[Scrutor.ServiceDescriptor(typeof(Vehicle), ServiceLifetime.Scoped)]
public class Vehicle : IVehicle

New Block

[Scrutor.ServiceDescriptor(typeof(IVehicle), ServiceLifetime.Scoped)]
public class Vehicle : IVehicle

@krishnan1159
Copy link
Author

@khellang What is the scan order in which classes will be scanned? Will the inherited classes will be scanned first and then parent classes will be scanned? Can you please throw some light on it?

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

2 participants