jamesmh / coravel

Near-zero config .NET library that makes advanced application features like Task Scheduling, Caching, Queuing, Event Broadcasting, and more a breeze!
https://docs.coravel.net/Installation/
MIT License
3.66k stars 242 forks source link

Is it possible to queue an invocable with arguments? #62

Closed chrisrichards closed 5 years ago

chrisrichards commented 5 years ago

Thanks for this really useful, and simple to use library!

I'd like to be able to queue an invocable that accepts a parameter which is then available in its Invoke method.

e.g. something like this

Queue the invocable:

_queue.QueueInvocable<SalesOrderProcessor>(salesOrderId);

Invocable implementation:

public class SalesOrderProcessor : IInvocable
{
    public Task Invoke(int salesOrderId)
    {
        // load sales order e.g.
        //   var salesOrder = await _context.SalesOrders.FindAsync(salesOrderId);

        // process sales order
    }
}

I can't see that this is possible at the moment, or have I missed something?

jamesmh commented 5 years ago

Thanks @chrisrichards for the kind words!

You are correct, invocable classes cant accept parameters when using the syntax in your example.

A working example might be something like this:

  1. Change your invocable to accept the salesOrderId as a constructor arg.
public class SalesOrderProcessor : IInvocable
{
    private int _salesOrderId;

    public SalesOrderProcessor(int salesOrderId)
    {
        this._salesOrderId = salesOrderId;
    }

    // Don't forget the async keyword ;)
    public async Task Invoke(int salesOrderId)
    {
        var salesOrder = await _context.SalesOrders.FindAsync(this._salesOrderId);

        // Do your stuff.
    }
}
  1. Then queue it up like this:
var invocable = new SalesOrderProcessor (salesOrderId);
_queue.QueueAsyncTask(async () => await invocable.Invoke());

// This might work as a shortcut (I'm just writing all this off the top of my head)
_queue.QueueAsyncTask(invocable.Invoke);

Let me know if that works 👍

jamesmh commented 5 years ago

@chrisrichards Keep in mind that the sample above would mean you can't really schedule your invocable class - which you might not need anyways?

chrisrichards commented 5 years ago

The SalesOrderProcessor has dependencies of it's own, so I'm just trying out:

  1. SalesOrderProcessor - no longer invocable

    public class SalesOrderProcessor
    {
    private readonly DbContext _context;
    
    public SalesOrderProcessor(DbContext context)
    {
        _context = context;
    }
    
    public async Task Process(int salesOrderId)
    {
        var salesOrder = await _context.SalesOrders.FindAsync(this._salesOrderId);
    
        // Do your stuff.
    }
    }
  2. Queue it

// _processor is DI injected SalesOrderProcessor in the constructor
_queue.QueueAsyncTask(() => await _processor.Process(salesOrderId));

Is there any reason why a typed IInvocable is not present? Looking through the code, it looks like it should be possible to have an IInvocable where type T is the parameter type. The queue and scheduler would need to be updated to add methods that accept IInvocable.

jamesmh commented 5 years ago

The reason that an Invocable type that accepts a parameter doesn't exist is that Invocables are a ubiquitous type that can be queued, scheduled, etc. across the board. So they are designed to encapsulate an entire isolated "job".

In your case, this is exactly why the Events feature exists.

You could create an event SalesOrderCompleted (or whatever the context is) that would be broadcasted to one or many listeners. One of them being ProcessSalesOrder (which could use a more specific name).

Event

public class SalesOrderCompleted: IEvent
{
    public int SalesOrderId { get; set; }

    public SalesOrderCompleted(int salesOrderId)
    {
        this.SalesOrderId = salesOrderId;
    }
}

Listener

public class ProcessSalesOrder: IListener<SalesOrderCompleted>
{
    private DbContext _dbContext;

    public ProcessSalesOrder(DbContext dbContext){
        this._dbContext = dbContext;
    }

    public async Task HandleAsync(SalesOrderCompleted broadcasted)
    {
        var salesOrder = await _context.SalesOrders.FindAsync(broadcasted.SalesOrderId);
    }
}

Broadcast

await _dispatcher.Broadcast(new SalesOrderCompleted(salesOrderId)); 

The only downside to this is it won't be queued.

In the event that you need it queued (it's a weird workaround) - create an invocable that launches this event (and would have the dispatcher injected into it's constructor).

This is a lot of work though - so I think your use case brings up a valid issue.

Should there be a new type? Or just a way to queue event broadcasting?

Or both? lol

jamesmh commented 5 years ago

@chrisrichards I added a new feature in version 2.1.0 that you can check out here.

This addresses the type of scenario you described. Use the example in my previous comment but instead of dispatching the event with IDispatcher you can queue an event to be broadcasted in the background:

this._queue.QueueBroadcast(new SalesOrderCompleted(salesOrderId));
chrisrichards commented 5 years ago

Awesome! Will take a look when I can, thanks.

oltronix commented 4 years ago

Hi, not sure if this necromancy is allowed or if I should create a new ticket. But I like the idea of being able to pass parameters to a job to modify the execution so just wanted to bounce this idea.

I think something like this could support parameters to Invocables without changing the base behavior, I tried it out in a clone, if you would like to take a look I could do a PR.

public interface IInvocable<TParam> : IInvocable
        where TParam : class
    {
        /// <summary>
        /// Execute the logic/code for this invocable instance with the provided TParam.
        /// </summary>
        /// <param name="parameter">A class with parameters for this invocable.</param>
        /// <returns></returns>
        Task Invoke(TParam parameter);

        /// <summary>
        /// This should be a instance of TParam that is valid for running the Invoke method.
        /// This will be provided as a paramter when the Invocable is scheduled without parameters or the parameter value is null.
        /// </summary>
        /// <returns></returns>
        TParam Default { get; }
    }

The user would have to provide a default parameter that is used when you schedule the job normally, then the Queue and Schedule could add new methods like: public IScheduleInterval ScheduleWithParameter<T, TParam>(TParam parameter) void QueueInvocableWithParameter<T, TParam>(TParam param) Giving flexibility without making any breaking changes.

This would allow us to do things like this:

scheduler.ScheduleWithParameter<NewTestInvocable, NewTestParam>(new NewTestParam { SomeVal = 2 })
      .EveryFiveSeconds();
scheduler.ScheduleWithParameter<NewTestInvocable, NewTestParam>(new NewTestParam { SomeVal = 3 })
      .EveryThirtySeconds();
scheduler.Schedule<NewTestInvocable>()
      .EveryTenSeconds();
jamesmh commented 4 years ago

Right now, you could do something like this:

scheduler.ScheduleAsync( async scheduler =>
     var job = new MyJob();
     job.WithParam(new TestParam(5)); // Just a method you added to your invocable;
     await job.Invoke();
);

Or use events like mentioned in this thread.

But I get that the syntax you supplied is more straightforward. Will have to mull on that a bit 😋