Added support for full connection string passed to constructor or con #168

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

CreepyGnome commented Dec 17, 2012

Allows a full connection string to be passed to the constructor while
still supporting the connection string name from config file. This
allows for environments or situations where you don't use the a config
file. Also renamed ConnectionString to _connectionString to be inlined
with visible naming conventions in the file, which is also in line with
common conventions for C#.

Rodney Foley added some commits Dec 17, 2012

Added support for full connection string passed to constructor or con…
…nection string name

Allows a full connection string to be passed to the constructor while
still supporting the connection string name from config file. This
allows for environments or situations where you don't use the a config
file. Also renamed ConnectionString to _connectionString to be inlined
with visible naming conventions in the file, which is also in line with
common conventions for C#.
Moved GetType call out of if blocks
Moved GetType call out of the if blocks which was calling it up to 4
times, now it is only called once always.
Revert "Added support for full connection string passed to constructo…
…r or connection string name"

This reverts commit 35a9e2c.
Revert "Revert "Added support for full connection string passed to co…
…nstructor or connection string name""

This reverts commit 9d91837.
Contributor

MikeBeaton commented Mar 15, 2017

The original version of this commit 35a9e2c looks like quite a useful change (i.e. if a connection string with the passed-in connectionStringName does not exist, use the passed-in string as an actual connection string instead). I was wondering if anyone reading this had any insights into why/not to do it?

On a closely related vein, I've been looking through all past issues for discussion of support for ProviderName in Massive (https://github.com/FransBouma/Massive/issues?q=providername) and was wondering:

  • Is the hard-coded DbProviderFactoryName (and the earlier hard-coded equivalents in v1.0) really needed?
    • Noting that Massive always works with an IConnectionStringProvider then (I am assuming, but I am not sure, that) even in non-config-file-based providers, there is always the potential to correctly support GetProviderName() from the non-config-file config source. (Or, at the very least, that this could be hard-coded by the user into their own implementation of IConnectionStringProvider.)
  • Is DB.Current really general enough to be useful?
    • Noting that you can always do db = new DynamicModel(connectionStringName) to get the same result.
    • There are many install examples (including the default Oracle and MySQL installs) in which machine.config does NOT contain exactly one connection string (the assumption of the DB.Current code, as per #211), so DB.Current will just fail in these setups (which is presumably always for anyone on Oracle or MySQL).
    • And finally, pace #153, I believe it would NOT be obvious to many people who just pick up Massive and want to start using it that (and exactly why) DB.Current is fragile like this.

I do fully recognise that with respect to both of my two questions just above, any change would break the existing API. But these might - just possibly - be changes that could be carefully handled in some new version of Massive? I am thinking, e.g., in terms of throwing a NotSupportedException if these features are ever used, but keeping the relevant legacy code still there for commenting back in by the user, for the few(?) currently relying on either of these, and not willing or able to make the simple changes needed (to their config, or code, respectively) to stop using them, once they see the warning. (None of the existing tests touches either of these......)

Contributor

MikeBeaton commented Mar 15, 2017

To be completely clear, the possible problems here (which aren't urgent, and don't need fixing now, but which I perceive as real) are:

  • Users have to make a code change to completely cleanly switch ADO.NET providers with a given database (otherwise you're using one provider but you've still got a hard-coded reference to another one, and this might get accessed silently by mistake rather than giving a meaningful warning, e.g. if you omit ProviderName in your config file on one connection string). #240
  • Massive is shipped with a fragile DB.Current implementation which will fail fairly nastily and mysteriously for many (especially non-SQL Server) users.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment