HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Multiple connection string in Web API 2

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
multiplewebapistringconnection

Problem

I have a Web API 2 using Dapper.net, and it's accessed by various client with their own database. Currently, I'm using custom header called Client-Id to identify the client.

This Client-Id contains a ConnectionString name and I passed this connection string into my constructor.

Here's my project structure:

Api.Persentation -> Api.Service -> Api.Logic


The connection string will pass from Api.Persentation to Api.Logic as follows:

Api.Persentation

public string testApi()
{
//using the service
//consider the connection string was handled on base class
var _myService = new testApiService(base.ConnectionString); 

return _myService.HelloWorld();
}


Api.Service

public class testApiService: BaseService
{
    #region const
    //This class was from API.Logic
    private readonly testApiRepo _testApiRepo;

    //the connectionString passed into base class to use a log repo
    public testApiService(string connectionString)
        : base(connectionString)
    {
        this._testApiRepo = new testApiRepo(connectionString);
    }
    #endregion

    public string HelloWorld()
    {
        //access API.Logic
        return this._testApiRepo.GetHelloWorld();
    }
}


Api.Logic

It has two classes, where BaseConnection is an abstract class to open the connection (the class how I use ConnectionString). The second one is, testApiRepo showing how I use the connection to get a database value.

```
/*-------------
BaseConnection Class
-------------*/
public abstract class BaseConnection
{
private string _connectionString;
public BaseConnection(string ConnectionString)
{
this._connectionString = ConnectionString;
}

public MyDbConnection GetOpenConnection(bool mars = false)
{
string cs = this._connectionString;
if (mars)
{
SqlConnectionStringBuilder scsb = new SqlConnectionStringBuilder(cs);
scsb.Multiple

Solution

First of all I would highlight a security problem: you're directly exposing your implementation to service users. They just need to try different Client-IDs to connect to different databases. Connection string should be deduced by your application logic from client's authentication (whatever you use for this).

testApi() does not follow common .NET naming guidelines, method names should be Pascal case: TestApi() (also I would consider to use a little bit more descriptive name, TestServiceConnection(), maybe). The same apply for testApiService (TestApiService) and your local variable _myService (prefixes such as _ are controversial but - when used - they denote member fields, not local variables or parameters).

Also classes testApiService and testApiRepo has same naming issue.

  • BaseConnection is abstract, you'd better also declare its constructor as protected.



  • _connectionString is initialized only in constructor then it should be marked as readonly.



  • ConnectionString constructor parameter should be camel case: connectionString.



In GetOpenConnection() you're using a boolean flag mars, they should be used sparely because reading GetOpenConnection(true) you don't immediately understand what it means (true...what?) and whenever possible they should be replaced with enum or different methods. I'd also consider to do not have a default value: do you really call it so often you want to save those few characters?

Be careful when you create and return disposable objects, if something goes wrong creating MyDbConnection (again it should be myDbConnection, also better name is required) object then an exception is thrown and connection isn't immediately destroyed: when a SQL connection is popped from pool is an implementation detail you should not rely on.

All your classes are public and none of them are sealed. Will they be used outside assembly where you declared them? If not then they should not be public. Also TestApiRepo should be renamed and marked as sealed.

There isn't much more I can say about these snippets but I think you should change little bit your architecture. For example what you call BaseConnection in reality is not a connection! It's a factory class where you build connection objects (MyDbConnection) and testApiRepo has nothing to do with that factory (it should not be a derived class) but, instead, accept factory as parameter (or a connection string and then instantiate your ConnectionFactory class).

Context

StackExchange Code Review Q#116842, answer score: 3

Revisions (0)

No revisions yet.