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

Add possibility to use dynamically loaded assemblies #5352

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

truthz03
Copy link

@truthz03 truthz03 commented May 8, 2024

Because I need a way to load assemblies dynamically (plugins) in my project, I found some problems while serializing/deserializing payloads.
In order to solve this problem, I would like to implement a mechanism to resolve the necessary types manually if needed.


This change is Reviewable

@truthz03
Copy link
Author

truthz03 commented May 8, 2024

@dotnet-policy-service agree

@cvijayak
Copy link
Contributor

cvijayak commented May 13, 2024

@truthz03

Please confirm whether the commit "367f574" will resolve the bug "#5356"

@truthz03
Copy link
Author

@cvijayak
No, my commit will not fix this bug. My commit gives the possibility to deserialize stored things into unknown objects.
This means, if I add some activities and there payloads as plugin (within a different AppDomain), it will be possible to deserialize the json content to a type only known by the plugin AppDomain.

The bug #5356 shows problems with the serialization. It throws an error because the type is System.Type, which is a known type. I also don't know why System.Type is not serializeable.

@truthz03
Copy link
Author

Is there something to do for my to get the "SonarCloud Code Analysis" ready?
Or have I still to wait?

@sfmskywalker
Copy link
Member

@truthz03 Thanks for your PR! At this point there's nothing for you to do, I just need time to review and test. Thank you for your patience.

@truthz03
Copy link
Author

@sfmskywalker Hi, can you tell me how long it will take to get my PR merged?

@sfmskywalker
Copy link
Member

@truthz03 Hey there, sorry it's been taking a while. I will get this merged this week. Thanks again for your contribution, your PR makes sense!

@sfmskywalker sfmskywalker added this to the Elsa 3.2 milestone Jun 13, 2024
@sfmskywalker sfmskywalker force-pushed the enhancement/allow-to-use-dynamically-loaded-assemblies branch from 30603b0 to f6aa105 Compare June 20, 2024 21:19
@sfmskywalker
Copy link
Member

@truthz03 Unfortunately, the main branch received an update that uses the PolymorphicObjectConverterFactory as a type in a JsonConverterAttribute - which only works with parameterless constructors.

In other words, we need to revisit either that change, or your change, such that you can still resolve services from DI.

@truthz03
Copy link
Author

@sfmskywalker Ok. Let me know if you find a solution.
Maybe its possible to have 2 constructors. One parameterless for the attribute where it is not possible to inject anything and one with parameters.

@sfmskywalker sfmskywalker modified the milestones: Elsa 3.2, Elsa 3.3 Jul 18, 2024
@sfmskywalker sfmskywalker removed this from the Elsa 3.3 milestone Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

3 participants