Validate table prefix before allowing new tenant to be added

[MikeAlhayek]

Describe the bug

When adding a new tenant, we allow the user to set a prefix to the tables to enable reusing the same database for multiple tenets. However, we don't validate to ensure that the prefix isn't already used first. The problem with this is that it throws an exception

To Reproduce

Steps to reproduce the behavior:

1. Create 2 tenant on same database connection with the same prefix or both with no prefix.
2. Set up both tenants. you should see the exception when you set up the second tenant.

Suggestion

When creating the tenant, we should group existing connections by prefix. Then check to make sure the prefix isn't already used before allowing it.

I think this logic should be part of the ITenantValidator But I think the validation would have to be done per type/servername/databasename/prefix

[hishamco]

I remembered @jtkech mentioned the reason behind "Invalid Serial Number" long time ago, may be the tenant not loaded successfully

[Skrypt]

Maybe this should be part of YesSql, it needs to be evaluated.

[MikeAlhayek]

@Skrypt I agree that YesSql should provide an easy way to get info about the connection.

We could add a new services called IConnectionFactoryProvider that would allow us to create DbConnection for a given provider/connection string. Here is an example "not tested"

public interface IConnectionFactoryProvider
{
	public IConnectionFactory GetFactory(string providerName);
	public IConnectionFactory GetFactory(ShellSettings settings);
}

public class ConnectionFactoryProvider : IConnectionFactoryProvider
{
	public IConnectionFactory GetFactory(string providerName, string connectionString)
	{
		ArgumentNullException.ThrowIfNull(providerName);
		ArgumentNullException.ThrowIfNull(connectionString);

		if(Is("SqlConnection", providerName))
		{
			return new DbConnectionFactory<SqlConnection>(connectionString);
		}

		if(Is("Sqlite", providerName))
		{
			return new DbConnectionFactory<MySqlConnection>(connectionString);
		}

		if(Is("MySql", providerName))
		{
			return new DbConnectionFactory<SqliteConnection>(connectionString);
		}
		
		if(Is("Postgres", providerName))
		{
			return new DbConnectionFactory<NpgsqlConnection>(connectionString);
		}

		throw new ArgumentOutOfRangeException("The provider '{0}' is not supported.", providerName);
	}

	public IConnectionFactory GetFactory(ShellSettings settings)
	{
		ArgumentNullException.ThrowIfNull(settings);

		return GetFactory(shellSettings["DatabaseProvider"], shellSettings["ConnectionString"]);
	}

	private bool Is(string name, string comparedTo)
	{
		return name.Equals(comparedTo, StringComparison.OrdinalIgnoreCase);
	}
}

The new IConnectionFactoryProvider service would be registered from OrchardCoreBuilderExtensions

Another cleaner way, we could add the following parameter to DatabaseProvider class public Func<string, IConnectionFactory> GetFactory { get; set; }

Then we can define the factory during registration using something like this

services.TryAddDataProvider(name: "Sql Server", value: "SqlConnection", hasConnectionString: true, sampleConnectionString: "Server=localhost;Database=Orchard;User Id=username;Password=password", hasTablePrefix: true, isDefault: false,  (connection) => new DbConnectionFactory<SqlConnector>(connection));

Then simply, we can find the DatabaseProvider instance and call the provider.GetFactory(_shellSettings["ConnectionName"]) where provider is something we can find by evaluating IEnumerable<DatabaseProvider>

However, with the second approach we would have to add YesSql.Abstraction dependency to OrchardCore.Data.Abstractions which may not be acceptable.

[hishamco]

Maybe this should be part of YesSql, it needs to be evaluated.

Let me check YesSQL I'm sure I suggested or mentioned something similar there

[hishamco]

The thing that I refered to related to checking the connection string if it's valid, but here the case is different

[hishamco]

Seems ValidateAsync() is the right place to do this check

OrchardCore/src/OrchardCore.Modules/OrchardCore.Tenants/Services/TenantValidator.cs

Line 38 in ef76b6f

public async Task<IEnumerable<ModelError>> ValidateAsync(TenantViewModel model)

@CrestApps I can submit a PR for this, unless you need to do it ;)

[hishamco]

I think this logic should be part of the ITenantValidator But I think the validation would have to be done per type/servername/databasename/prefix

I didn't notice that you refer to ITenantValidator before ;)