Grace icon indicating copy to clipboard operation
Grace copied to clipboard

Attribute [Export] is exported twice

Open jods4 opened this issue 5 years ago • 10 comments

Here's my config, I think everything except the last line is irrelevant to the issue at hand:

      var container = new DependencyInjectionContainer(c => c.AutoRegisterUnknown = false);
      container.Configure(c =>
      {        
        // Register one ILogger per container with automatic job context
        c.ExportFactory((IExportLocatorScope scope) => Log.ForContext("Job", scope.ScopeName))
         .Lifestyle.SingletonPerScope();

        c.ExportFactory(A).AsKeyed<IDbConnection>("a");
        c.ExportFactory(B).AsKeyed<IDbConnection>("b");

        c.AddInspector(new ConfigInspector());

        // [Export] from this assembly
        c.ExportAssemblyContaining<JobSchedulerService>().ExportAttributedTypes();
      });

ExportAttributedTypes does export all classes in my assembly that have [Export], or [ExportByInterfaces], etc.

But when I look at activation strategies, or simply at WhatDoIHave output, I notice that all [Export] exports are actually exported twice. They have the AsType: XY line listed twice for every type they are exported as.

jods4 avatar Sep 13 '19 16:09 jods4

Interesting. I'll have to take a look this weekend.

ipjohnson avatar Sep 13 '19 17:09 ipjohnson

@ipjohnson I was reading the code trying to see if I can figure out why. This is a complex codebase so I might be completely off... but could it be:

  1. CreateExportStrategiesForTypes will process the types found in my assemblies. Notice that this method looks for IExportAttribute and will add those types to exportTypes right here: https://github.com/ipjohnson/Grace/blob/39d4afb781a4b3b1634d33c6fd6b1165aac633a7/src/Grace/DependencyInjection/Impl/ExportTypeSetConfiguration.cs#L357 Because we have types in exportTypes, the method will then call CreateExportStrategyForType, here: https://github.com/ipjohnson/Grace/blob/39d4afb781a4b3b1634d33c6fd6b1165aac633a7/src/Grace/DependencyInjection/Impl/ExportTypeSetConfiguration.cs#L373

  2. CreateExportStrategyForType does two unfortunate things. It exports everything in exportTypes, right here: https://github.com/ipjohnson/Grace/blob/39d4afb781a4b3b1634d33c6fd6b1165aac633a7/src/Grace/DependencyInjection/Impl/ExportTypeSetConfiguration.cs#L402 I think this might be our first AsType export Later it goes on to process attributes here: https://github.com/ipjohnson/Grace/blob/39d4afb781a4b3b1634d33c6fd6b1165aac633a7/src/Grace/DependencyInjection/Impl/ExportTypeSetConfiguration.cs#L421

  3. I'm skipping a few indirections and intermediate methods. The default IActivationStrategyAttributeProcessor will end up right here, inside ProcessClassAttributes: https://github.com/ipjohnson/Grace/blob/e6853c7fb3b23a27824b9b2e17ad860b9b4a403b/src/Grace/DependencyInjection/Impl/ActivationStrategyAttributeProcessor.cs#L126 I think this is where we are AsType exported for the second time

Maybe I found the reason why attributes are exported twice. I am not sure what the fix is, though.

Gut feeling is that exportTypes at step 1 should not conflate exported types and types that are of interest because they have attributes.

jods4 avatar Sep 19 '19 20:09 jods4

That does look to be a problem but my concern would be changing some logic right now and introducing a new bug.

I've been thinking maybe the answer is to check and make sure that duplicate types aren't being added to the the export type list.

ipjohnson avatar Sep 20 '19 12:09 ipjohnson

It's more dirty but you could post-process the registrations and dedup identical exports... the result from user perspective would be the same.

jods4 avatar Sep 20 '19 12:09 jods4

Friendly bump! 🧸 I'm starting a new project with more or less the same auto-discovery setup, wondering if you had a chance to think about this bug.

jods4 avatar May 26 '20 11:05 jods4

I've added some duplicate checking for ExportAs at the strategy level. Could you try it on the nightly nuget feed https://ci.appveyor.com/nuget/grace-master ?

ipjohnson avatar May 26 '20 22:05 ipjohnson

I did a quick test and I can confirm:

  • with 7.1 AsType export are still duplicated;
  • with 7.2-beta800 they're not.

Other than that my project seem to pass smoke tests.

jods4 avatar May 27 '20 16:05 jods4

I'll push this to NuGet this weekend along with the attribute usage change we talked about

ipjohnson avatar May 27 '20 21:05 ipjohnson

Hi @ipjohnson !

This is old but I'd like to let you know that we use 7.2 in my projects and as reported in my previous comment exports are not duplicated anymore, so I think you can close this issue.

Thanks a lot and I wish you a happy new year!

PS: I noticed you're working on a big 8.0 release, are there release notes with the main changes somewhere?

jods4 avatar Jan 04 '23 14:01 jods4

Hi @jods4

Version 8 is about removing deprecated code and deprecated targets, I'll put together list of changes this weekend.

ipjohnson avatar Jan 06 '23 01:01 ipjohnson