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 an extension point for customizing establishing of ADO.NET connections #87

Merged

Conversation

a-belevich
Copy link
Contributor

Make usage of NHibernate compatible with SET NOCOUNT ON DB-wide setting #112 issue shows how NOCOUNT server setting can break the NHibernate persistence. It was solved by injecting NHibernate's DriverConnectionProvider, that fixes connections immediately after their opening. However, when SQL Server transport is used, and the same database is used for Timeouts persistence and SQL Transport, it can happen that connections opened for transport can be used by Timeouts persistence routines.

Solution: allow injection of custom SQL connections' factories, like NHibernate does. Such factories can, for example, send SET NOCOUNT OFF command immediately after opening the connection.

@ramonsmits
Copy link
Member

@a-belevich Thanks for the effort! There are a couple of improvements before we can merge this.

Having an API like:

public void Configure()
{
    BusConfiguration busConfiguration = new BusConfiguration();
    busConfiguration
        .UseTransport<SqlServerTransport>()
        .UseCustomConnectionCreationMethod(CreateSession);
}

static SqlConnection CreateSession(string connectionString)
{
    var conn = new SqlConnection(connectionString);
    conn.Open();
    return conn;
}

@a-belevich a-belevich force-pushed the fix/develop/sqlconnectionfactories branch from a509bab to 6896e74 Compare August 11, 2015 17:49
@a-belevich
Copy link
Contributor Author

@ramonsmits Thank you for review. New version uses injection instead of static factory class. Now configuration looks like

    configuration
        .UseTransport<SqlServerTransport>()
        .UseCustomSqlConnectionFactory(NoCountSafeSqlConnectionFactory.OpenNewConnection);

where factory is of type Func<string, SqlConnection>.

@SzymonPobiega Ramon mentioned that he's going to vacation. If it's still true, could you please take a look?

@a-belevich a-belevich force-pushed the fix/develop/sqlconnectionfactories branch from 6896e74 to 8dec49a Compare August 11, 2015 18:41
…ting Particular#112

- Allow customizing of SQL Transport connections with injectable factory.
- Unit tests for SqlConnectionFactoryConfig
@a-belevich a-belevich force-pushed the fix/develop/sqlconnectionfactories branch from 8dec49a to d860404 Compare August 11, 2015 21:17
@SzymonPobiega
Copy link
Member

@a-belevich looks good. One thing that I would change is registration of Func<string, SqlConnection>. I would wrap that in a private class e.g. ConnectionFactory. The reason is, it would be more explicit in the class that use it looking just on a type what a constructor argument is for. Also, there is a chance (small but...) that registraction of a Func<string, SqlConnection> might clash with another feature wanting to register same type.

Do you have time to do this small adjustment? If no, I can do it after merging. Any way, thanks for the contribution!

@a-belevich a-belevich force-pushed the fix/develop/sqlconnectionfactories branch from a5af82d to 30c216f Compare August 20, 2015 00:18
@a-belevich
Copy link
Contributor Author

@SzymonPobiega Thank you for reviewing. I've added CustomSqlConnectionFactory class that wraps the Func in container. Is that the change you've meant?

SzymonPobiega added a commit that referenced this pull request Aug 20, 2015
…ries

SqlConnections should be created with factory
@SzymonPobiega SzymonPobiega merged commit 28a610d into Particular:develop Aug 20, 2015
@SzymonPobiega
Copy link
Member

@a-belevich thanks a lot! Merged.

@SzymonPobiega SzymonPobiega self-assigned this Sep 1, 2015
@SzymonPobiega SzymonPobiega added this to the 2.2.0 milestone Sep 1, 2015
@SzymonPobiega SzymonPobiega changed the title SqlConnections should be created with factory Add an extension point for customizing establishing of ADO.NET connections Sep 24, 2015
@particularbot particularbot removed this from the 2.2.0 milestone Oct 29, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants