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 DI Hosted commands #93

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

tgardner
Copy link
Contributor

Fixes #92

@tgardner tgardner marked this pull request as ready for review August 19, 2024 12:23
@jeremydmiller jeremydmiller merged commit 0134fb9 into JasperFx:master Sep 4, 2024
1 check passed
@jeremydmiller
Copy link
Member

jeremydmiller commented Sep 4, 2024

@tgardner Shoot, I took this in too quickly. It's going to take some changes for disposal at a minimum.

This:

internal class DependencyInjectionCommandCreator : ICommandCreator
{
    private readonly IServiceProvider _serviceProvider;
    public DependencyInjectionCommandCreator(IServiceProvider serviceProvider)
    {
        _serviceProvider = serviceProvider;
    }

    public IOaktonCommand CreateCommand(Type commandType)
    {
// Right here, you need to do something to ensure that the scope is disposed to clean up stuff
        var scope = _serviceProvider.CreateScope();
        return (IOaktonCommand)scope.ServiceProvider.GetRequiredService(commandType);
    }

    public object CreateModel(Type modelType)
    {
        return Activator.CreateInstance(modelType)!;
    }
}

I'll build off what you did here as a starting point though.

@tgardner
Copy link
Contributor Author

tgardner commented Sep 4, 2024

@jeremydmiller - I was under the belief that the scope would need to live as long as the command that it created did.
Disposing the scope, will in turn dispose any objects created, resulting in an exception when trying to use inside the command.

I made some assumptions here, possibly wrong. That the scope would be disposed of once the command executes and application finishes.

I'm seeing a couple of options here:

  1. Currently it's creating a scope per command. This was to ensure that each command isn't populated with any shared scoped resource. We'd need a mechanism to tie the disposal of the scope to the disposal of the command.
  2. Create a single scope for the command creator. Move the scope creation to the constructor and dispose of it along with the CommandCreator. I don't like this as much, as in the case of DbContexts etc, you'll end up with potentially change tracked entities already present in command initialization.

I'm happy to help if you can provide any form of guideance.

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

Successfully merging this pull request may close these issues.

Add support for dependency injection and new IHostApplicationBuilder syntax
2 participants