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

Simplify config file-based connection string configuration #39

Closed
dmitry-a-morozov opened this issue Jan 28, 2014 · 9 comments
Closed
Labels

Comments

@dmitry-a-morozov
Copy link
Member

Currently SqlCommanProvider allows to configure connection string in similar way as built-in SqlDataConnection or SqlEntityConnection providers:

  1. via ConnectionString parameter supplying connection string literal
  2. via ConnectionStringName parameter providing name of connection string from config file.
    Initially I liked this design better that Entity Framework version where single parameter to DbContext class ctor plays dual role - it can be either connection string literal or name.
    http://msdn.microsoft.com/en-us/library/gg679467(v=vs.113).aspx
    http://msdn.microsoft.com/en-us/data/jj592674.aspx

Two distinct parameters design works fine for both SqlDataConnection or SqlEntityConnection providers because connection string information can be shared between root data context type and all internal types. This model breaks down for SqlCommandProvider because different type instances of SqlCommand<...> cannot share any information. Imagine 20-30 queries defined in an application all configured via config file.

type MyCommand1 = SqlCommand<...,ConnectionStringName = AdventureWorks">
type MyCommand2 = SqlCommand<...,ConnectionStringName = AdventureWorks">
...
type MyCommandN = SqlCommand<...,ConnectionStringName = AdventureWorks">

This is a lot of typing !!!
To make things worse ConnectionStringName has to be used as named parameter where ConnectionString can be passed by position

type MyCommand = SqlCommand<..., connStr>

It is also confusing that both ConnectionString and ConnectionStringName parameters are optional because there is no way to express that either one has to be supplied.

Considering all above I'm inclined to switch to EF-like design with one mandatory string parameter which can be either string literal or "name=VALUE" (value is name of connection string from config file). It will be possible to re-write the example above like following:

[]
let connectionString = "name=AdventureWorks"

type MyCommand1 = SqlCommand<...,connectionString >
type MyCommand2 = SqlCommand<...,connectionString >
...
type MyCommandN = SqlCommand<...,connectionString >

@tachyus-dev
Copy link

I'm not very familiar with EF (and I hope SqlCmdProv continues to make it unnecessary for me to learn).

If I understand correctly currently in SqlCommandProvider:

either SqlCommand<... ConnectionStringName="blah" ...>

or SqlCommand<... ConnectionString="foo" ...>

and you are proposing a change to SqlCommand<... ConnectionInfo="(name= if a name, otherwise string)" ...>

Is this correct? And somehow a connection string can be shared between types so it is unnecessary to add it to every type, but if you want to override, either by name or string you can?

So far, working with Azure Websites, I want to use Name in all cases. If there were a way to set default name, and only override name when necessary, that would be advantageous (perhaps), otherwise ConnectionString is actually rather useless.

@jackfoxy
Copy link

@tachyus-dev my alter-ego

@dmitry-a-morozov
Copy link
Member Author

Yes, I think you understood me correctly.
You be able to define string literal

[]
let conn = "name=AdventureWorks"

And use it to define commands either using shorter positional version
type MyCommand1 = SqlCommand<..., conn, ..>

or named parameters
type MyCommand1 = SqlCommand<..., ConnectionStringOrName = conn, ...>

Run-time override can be provide either via config file or some kind of factory that knows to pass down correct connection string to SqlCommand constructor.

I hope it will reduce typing.
In the latest version ConnectionStringName has default value "SqlCommandProvider" but I don't think it's good idea. Non-intuitive. With the new design it will be easy to define string literal and share it between multiple command definitions.

@jackfoxy
Copy link

Marginal improvement from my perspective, but the big win would be in completing docs and losing "experimental" from the project path :)

The way I operate it wouldn't save much typing because this is the kind of thing I repeatedly copy and paste a template and then add finishing touches.

@dmitry-a-morozov
Copy link
Member Author

I understand it's minor improvement but still worth an effort. Those innocent copy-and-paste templates can get out of hand quickly.
I wanted to confirm it with you because it will be a breaking change though easy to fix.
Yes, I'm working on docs and trying to wrap up the project so it can be moved to release mode.
Let me talk to Don about Experimental namespace.
Also I want to improve connection string configuration via constructor too - customers complain it's far from optimal experience.

@jackfoxy
Copy link

Should be just a minor change for us.

@ghost
Copy link

ghost commented Jan 28, 2014

Go ahead and drop Experimental

@vasily-kirichenko
Copy link
Contributor

The simplified type declaration is definitely good, but it does not solve the massive code duplication as we start passing the same runtime connection string to all the commands' constructors (I assume we use runtime connection string that's retrieved not from App.config).

I suggest generate Create static member for all commands like this:

type SqlCommand(connectionString: string) =
    static member Create connectionString = SqlCommand connectionString

Then we'll able to write a generic factory, similar to this one:

module DB =
    let inline get< ^a when ^a: (static member Create: string -> ^a)> =
        (^a: (static member Create: string -> ^a) MySettings.Default.Databases.Database1)

Usage:

[<Literal>]
let DB1 = "Data Source=localhost; Initial Catalog=DB; Integrated Security=True"

type Proc1 = SqlCommand<"dbo.Proc1", DB1, CommandType = CommandType.StoredProcedure>
type Proc2 = SqlCommand<"dbo.Proc2", DB1, CommandType = CommandType.StoredProcedure>
type Proc3 = SqlCommand<"dbo.Proc3", DB1, CommandType = CommandType.StoredProcedure>

DB.get<Proc1>.Execute(...)
DB.get<Proc2>.Execute(...)
DB.get<Proc3>.Execute(...)

@dmitry-a-morozov
Copy link
Member Author

release 1.1.27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants