rebus-org / Rebus.SqlServer

:bus: Microsoft SQL Server transport and persistence for Rebus
https://mookid.dk/category/rebus
Other
44 stars 45 forks source link

Add new RebusConfigurer.Outbox StoreInSqlServer method overload that takes in Func<Task<IDbConnection>> connectionFactory instead of string connectionString #100

Open robpackwood opened 1 year ago

robpackwood commented 1 year ago

For Sagas, I configure with syntax like this:

rebusConfigurer.Sagas(configurer =>
{
    configurer.EnforceExclusiveAccess();

    configurer.StoreInSqlServer(
        config.RebusConfig.PersistenceConnectionFactory,
        config.RebusConfig.SagaDataDbTable, config.RebusConfig.SagaIndexDbTable);
});

For Timeouts, I configure with syntax like this:

rebusConfigurer.Timeouts(configurer => configurer.StoreInSqlServer(
    config.RebusConfig.PersistenceConnectionFactory, config.RebusConfig.TimeoutsDbTable));

In both examples above StoreInSqlServer takes in Func for a connectionFactory parameter.

When configuring Outbox, the StoreInSqlServer() method is inconsistent with the methods mentioned above and could use an overload, which takes the same Func for a connectionFactory parameter. I would prefer to not store the database username and password in plain text for this connectionString and utilize the existing connectionFactory I already have in scope.

Current code configuring the Outbox settings:

rebusConfigurer.Outbox(configurer => configurer.StoreInSqlServer(
  config.RebusConfig.PersistenceConnectionString, config.RebusConfig.OutboxTableName));
mookid8000 commented 1 year ago

Oh yeah I can see that - it would of course be nice if the outbox configuration would accept the same connection factory as the other SQL Server configuration methods.

mookid8000 commented 1 year ago

OK I just took a quick stab at it - it wasn't readily possible to fully implement it, because the current design of the outbox requires access to the raw SqlConnection/SqlTransaction objects.

It's currently parked here: https://github.com/rebus-org/Rebus.SqlServer/tree/feature/outbox-connection-factory

Will maybe look into it some time next week.

Kraviecc commented 11 months ago

@mookid8000 do you think that it's possible to make IOutboxConnectionProvider public? For my use case (multi-tenant application) this is the last part to make it work (on the other hand, IOutboxStorage is publicly available).

EDIT: I meant only changing the modifier of the aforementioned interface, not considering these changes: https://github.com/rebus-org/Rebus.SqlServer/compare/master...feature/outbox-connection-factory. So, instead of configuring Outbox like this:

config.StoreInSqlServer(
    SqlConnectionString,
    TableName)

I could use a custom connection provider that dynamically resolves connection string and DB connection:

config.OtherService<IOutboxStorage>().Register(_ =>
    new SqlServerOutboxStorage(MyCustomConnectionProvider, TableName));

//This doesn't work - IOutboxConnectionProvider is internal
config.OtherService<IOutboxConnectionProvider>().Register(_ =>
    new MyCustomOutboxConnectionProvider());

EDIT2: I thought it behaves differently, but now I see other problems like outbox table initialization.