Scrutor icon indicating copy to clipboard operation
Scrutor copied to clipboard

Service Descriptor attribute is not working as expected

Open krishnan1159 opened this issue 3 years ago • 6 comments

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?

krishnan1159 avatar Sep 23 '22 12:09 krishnan1159

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 avatar Sep 23 '22 12:09 khellang

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

khellang avatar Sep 23 '22 12:09 khellang

@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 avatar Sep 23 '22 13:09 krishnan1159

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

krishnan1159 avatar Sep 23 '22 13:09 krishnan1159

@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 avatar Sep 23 '22 13:09 krishnan1159

@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?

krishnan1159 avatar Sep 23 '22 13:09 krishnan1159