testcontainers / testcontainers-dotnet

A library to support tests with throwaway instances of Docker containers for all compatible .NET Standard versions.
https://dotnet.testcontainers.org
MIT License
3.73k stars 262 forks source link

[Enhancement]: Make MsSqlBuilder.WithDatabase public #986

Closed vova-lantsov-dev closed 1 year ago

vova-lantsov-dev commented 1 year ago

Problem

MsSqlBuilder.WithDatabase is private for any reason, so you are really limited in what you can overwrite. The only currently available public method is WithPassword. There is private WithUsername as well, it's not that important, but can also be made public.

Solution

Make this method(s) public

Benefit

You can use a user-defined database name. It allows us to use some features that are not available for "system databases", such as change tracking on tables.

Alternatives

Implementing custom MsSqlBuilder that is the same as original but with public modifier on WithDatabase method.

Would you like to help contributing this enhancement?

Yes

DanielHabenicht commented 1 year ago

Its not configurable in the official docker image used. That is probably why they are private and not configurable.

However I would welcome a method for setting the Initial Catalog= Property in the connection string which should achieve the same as WithDatabase.

vova-lantsov-dev commented 1 year ago

@DanielHabenicht yeah, connection string modification sounds great

HofmeisterAn commented 1 year ago

Could someone help me comprehend and elucidate how the use case looks like when someone chooses to override the database name (Initial Catalog) and attempts to connect to the non-existent database using a basic SQL connection? This results in the following error:

Microsoft.Data.SqlClient.SqlException Cannot open database "foo" requested by the login. The login failed. Login failed for user 'sa'.

Which part is responsible for creating the database? How do developers know and be aware that they are responsible for database creation?

Taking into account that providing a connection string for a non-existent database is confusing and misleading, what prevents you from modifying the connection string and replacing the default database (using string replacement)?

In my opinion, it would be better to support the use case in the Microsoft SQL Server image and create an upstream issue. Microsoft should consider modifying the image to initialize the database by default.

This issue relates:

DanielHabenicht commented 1 year ago

@HofmeisterAn I have run into problems with calling .Migrate() to setup the database, which for some reason requires to set the database into SINGLE_USER mode which does not seem to be supported for master databases. So I need a database which is not master, thus using Inital Catalog=

HofmeisterAn commented 1 year ago

Simply changing the database value in the connection string without actually creating it will work in your specific example. This is due to the Entity Framework's capability to automatically generate the database if it doesn't exist. However, this approach will not work for other configurations, potentially leading developers to encounter issues. You can work around this utilizing e.g. the SQL connection string builder (while others cannot):

var connectionString = new SqlConnectionStringBuilder(_msSqlContainer.GetConnectionString());
connectionString.InitialCatalog = Guid.NewGuid().ToString("D");

In relation to this matter, I proposed several alternatives. One of these involves extending the MSSQL module's functionality to enable the execution of custom SQL scripts during startup, similar to PostgreSQL. This mechanism can then be harnessed to create a custom database or user, providing a versatile solution that accommodates various configurations.

It's important to note that my intention is to avoid providing an incorrect or invalid connection string for a module (or container) and relying on the assumption that the developer is using Entity Framework, which automatically generates the database upon initialization.

Furthermore, if the capabilities of the master database are limited (initializing the MSSQL container), I believe it would be better to provide this feedback to the upstream repository (Microsoft) rather than introducing vague behavior to Testcontainers.

vova-lantsov-dev commented 1 year ago

Similar for me, I use migrations and the change tracking feature on a database is required, but it cannot be set up on so-called "system databases", including master. EF Core will take care of database creation, so I really need only to set the Initial Catalog parameter.

1) Of course, I can use self-written logic to modify the connection string received from Testcontainers.MsSql 2) And I agree with providing a connection string for a non-existent database is confusing and misleading

Introducing the support of this feature on Microsoft side seems to be the best option so I close the issue here. Will think about reaching image maintainers. Thanks

HofmeisterAn commented 1 year ago

I would like to point out that if someone is interested in contributing and enhancing the capabilities of the Testcontainers MSSQL module to execute custom SQL scripts during startup (by overriding the entrypoint, etc.) and utilizing this feature to pre-create a database and user, I am completely open to that. However, I personally do not consider it a high priority.

vova-lantsov-dev commented 1 year ago

@HofmeisterAn I will take a look if I can help with that