search

weikio / PluginFramework

Everything is a Plugin in .NET
MIT License
557 stars 106 forks source link

FolderPluginCatalog: unnecessary assembly loading #44

Closed vasylmykytiukkryon closed 3 years ago

vasylmykytiukkryon commented 3 years ago

Hi, @mikoskinen.

Thank you for your great efforts on this, really appreciate it!

While testing this framework for my case, I found inconsistency with TypeFinderCriterias for FolderPluginCatalog. To be more precise, the problem is in FolderPluginCatalog.IsPluginAssembly method, as it uses the obsolete TypeFinderCriterias property to determine if the specified assembly contains plugins. https://github.com/weikio/PluginFramework/blob/master/src/Weikio.PluginFramework/Catalogs/FolderPluginCatalog.cs#L192 https://github.com/weikio/PluginFramework/blob/master/src/Weikio.PluginFramework/Catalogs/FolderPluginCatalog.cs#L248

As the result, if you configure FolderPluginCatalogOptions with the new TypeFinderOptions and not with the obsolete TypeFinderCriterias, it will treat all assemblies as those that contain plugins, which leads to unnecessary assembly loading.

I've already created a fix to this issue, but it seems that I don't have permission to push it. Could you please take a look?

Thanks, Vasyl

mikoskinen commented 3 years ago

Thank you for reporting this, and sorry for the delay 👍 I think the bug is clear, though if you happen to have a repro lying around, that is always useful when making sure that things are fixed :)

mikoskinen commented 3 years ago

Regarding your fix, if you have time, you could clone this repo, create the change there and do a PR. Alternatively you can also copy-paste the fix here.

mikoskinen commented 3 years ago

We have pushed a new 1.2.2 release to Nuget which should fix this issue. Let me know if you still encounter the problem.

vasylmykytiukkryon commented 3 years ago

Hi, @mikoskinen. I have already verified the fix and now it's working as expected :) Many thanks for your time and quick fix.

mikoskinen commented 3 years ago

Excellent, thanks for the info :)