1

Hi I am using Simple Injector and I am trying to implement a scheduler service which calls command(s) at a programmable regular frequency. My design decisions are below:

1) I have a service that runs a background thread and resolves from the container ICommandHandler<SyncExternalDataCommand>.Handle(someParameter) and calls the command handle method.

RegisterCommandAsBusinessUnitCronJob will be called for each BusinessUnit with the ID passed as a parameter on each call. We could also implement RegistercommandAsCustomerContactCronJob to loop through customer records.

Is it ok to design the scheduler in this way with a background thread?

2) I have tried to keep the coupling between the composition root container and the scheduler to a minimum.

An approach with Action helper delegates helps to keep the code in the composition root section as opposed to within the service. By using a different design, could this coupling be reduced further or is this coupling acceptable?

Code is below (it is yet to be made thread-safe), comments on the design decisions above and improvements/redesign are very welcome.

Thanks, Chris

Bootstrap of the container

void Register(Container container)
{
    container.RegisterSingle<IUnitOfWorkFactory>(
        new UnitOfWorkFactory(container));
    container.RegisterLifetimeScope<IUnitOfWork, UnitOfWork>();
    container.RegisterSingle<ILogger, Logger>();
    container.RegisterSingle<ICronJobService, CronJobService>();
    container.Register<ICommandHandler<SyncExternalDataCommand>, 
        SyncExternalDataCommandHandler>();

    var cronJobService = container.GetInstance<ICronJobService>();

    cronJobService.RegisterCommandAsBusinessUnitCronJob("* 1 * * *",
        (Int32 id) =>
        {
            var command = new SyncExternalDataCommand()
            {
                businessUnitId = id
            };

            using (container.BeginLifetimeScope()) 
            { 
                // handler must be resolved INSIDE the scope. 
                var handler =
         container.GetInstance<ICommandHandler<SyncExternalDataCommand>>(); 
                handler.Handle(command); 
            }
        }
    );
}

Scheduler service

// to be instantiated as a singleton by IoC container
public class CronJobService : ICronJobService
{
    // this dictionary is for tasks that will be called with 
    // the business unit primary key (Int32).
    private cronJobs = new Dictionary<Action<Int32>, string>();

    public void RegisterCommandAsBusinessUnitCronJob(
        string cronString, Action<Int32> commandFactory)
    {
        cronJobs.Add(commandFactory, cronString);
    }

    // this below is called when we are running a task
    protected static bool RunCreateAndHandleThread(
        object parameter)
    {
        var jobParameters = (ThreadJobParameters)parameter;

        if (!cancellationTokenSource.IsCancellationRequested)
        {
            // we will need to construct new graph with a
            // proxy command
            jobParameters.helper(jobParameters.businessUnitId);
        }

        return true;
    }

    protected static void SomeBackgroundLoop(
        RunHandlePollingLoopParameters parameters)
    {
        IUnitOfWork unitOfWork = 
            parameters.unitOfWorkFactory.CreateUnitOfWork();

        using (unitOfWork)
        {
            var businessUnits = 
                unitOfWork.BusinessUnitRepository.Get();

            // loop through cron jobs and business units
            foreach (var helperFactory in parameters.cronJobs.Keys)
            {
                // its time to run this job...
                if (isTimeToRunCronjob) 
                {
                    var jobParameters = new ThreadJobParameters
                    {
                        helper = helperFactory,
                        businessUnitId = businessUnit.Id
                    };

                    Task.Factory.StartNew<bool>(
                        RunCreateAndHandleThread, 
                        jobParameters, 
                        CronJobService.cancellationTokenSource.Token, 
                        TaskCreationOptions.LongRunning, 
                        TaskScheduler.Default);
                }
            }
        }
    }
}
Steven
  • 166,672
  • 24
  • 332
  • 435
morleyc
  • 2,169
  • 10
  • 48
  • 108
  • 2
    Please remember, the longer your question, the more unlikely that someone will take the time to study it. Could you shorten it to just the relevant bits? – stakx - no longer contributing Jun 18 '12 at 01:31
  • 1
    one of the developers of Simple Injector is somewhere around here, perhaps he sees this :-) – Mare Infinitus Jun 18 '12 at 08:10
  • @MareInfinitus: He is around here, but the question is a bit long :-( so I can't quickly answer the question. However, the question is rather general, and has little to do with the Simple Injector itself. So hopefully others can jump in as well. – Steven Jun 18 '12 at 08:15
  • 2
    Hi yes sorry for long post it has since been heavily edited and I have reduced it down to 2 questions, the second question is related to my now favourite IoC container specifically with the delegate `container.BeginLifetimeScope())` The code is working, and coupling seems acceptable, but since I am new to this new way of thinking if i am doing anything shocking please let me know - if it seems acceptable and doesnt have too much code smell then i am happy to run with it as-is. – morleyc Jun 18 '12 at 09:04

1 Answers1

2

Is it ok to design the scheduler in this way with a background thread?

Sure. As long as everything is thread-safe. From a DI perspective there is no problem, but of course you need to make sure that shared dependencies are thread-safe (as you must do with all multi-threaded applications). However, when the service is performant / fast enough without multi-threading, make the application single-threaded. This makes everything so much easier.

By using a different design, could this coupling be reduced further or is this coupling acceptable?

Possibly. Your CronJobService takes a dependency on the Container. This is not a problem, as long as it is part of the Composition Root. However, your CronJobService contains a lot of logic, which means you probably want to test it, but this is hard since you coupled it with the container and Task.Factory. Separate this if you can.

The use of a IUnitOfWorkFactory can be fine (take a look at this SO question and answer for instance). But since you already have a IUnitOfWork registered Per Lifetime Scope, I don't really see how a IUnitOfWorkFactory helps. So instead of calling the CreateUnitOfWork inside the SomeBackgroundLoop, you can just wrap this method with a using (container.BeginLifetimeScope()). This way this method runs inside the context of a lifetime scope, which means that any injected IUnitOfWork (within this thread) will be the same unit of work.

Community
  • 1
  • 1
Steven
  • 166,672
  • 24
  • 332
  • 435
  • 1
    again many thanks for your time and answers, SomeBackGroundLoop thread will spawn addititional threads, so for each thread a new object graph will need to be created in the new thread RunCreateAndHandleThread. Glad its seemingly on the right track, you should have seen my old service before i used IoC! – morleyc Jun 18 '12 at 13:29