Fix DbConnectionValidator when Shell Settings from database

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

@@ -91,7 +91,7 @@ public async Task<IEnumerable<ModelError>> ValidateAsync(TenantViewModel model)
                     }
                 }
 
-                await AssertConnectionValidityAndApplyErrorsAsync(model.DatabaseProvider, model.ConnectionString, model.TablePrefix, errors);
+                await AssertConnectionValidityAndApplyErrorsAsync(model.DatabaseProvider, model.ConnectionString, model.TablePrefix, errors, shellSettings?.IsDefaultShell() == true);
             }
             else
             {
@@ -100,25 +100,25 @@ public async Task<IEnumerable<ModelError>> ValidateAsync(TenantViewModel model)
                     // While the tenant is in Uninitialized state, we still are able to change the database settings.
                     // Let's validate the database for assurance.
 
-                    await AssertConnectionValidityAndApplyErrorsAsync(model.DatabaseProvider, model.ConnectionString, model.TablePrefix, errors);
+                    await AssertConnectionValidityAndApplyErrorsAsync(model.DatabaseProvider, model.ConnectionString, model.TablePrefix, errors, shellSettings?.IsDefaultShell() == true);
                 }
             }
 
             return errors;
         }
 
-        private async Task AssertConnectionValidityAndApplyErrorsAsync(string databaseProvider, string connectionString, string tablePrefix, List<ModelError> errors)
+        private async Task AssertConnectionValidityAndApplyErrorsAsync(string databaseProvider, string connectionString, string tablePrefix, List<ModelError> errors, bool isDefaultShell)
         {
-            switch (await _dbConnectionValidator.ValidateAsync(databaseProvider, connectionString, tablePrefix))
+            switch (await _dbConnectionValidator.ValidateAsync(databaseProvider, connectionString, tablePrefix, isDefaultShell))
             {
                 case DbConnectionValidatorResult.UnsupportedProvider:
                     errors.Add(new ModelError(nameof(TenantViewModel.DatabaseProvider), S["The provided database provider is not supported."]));
                     break;
                 case DbConnectionValidatorResult.InvalidConnection:
                     errors.Add(new ModelError(nameof(TenantViewModel.ConnectionString), S["The provided connection string is invalid or server is unreachable."]));
                     break;
-                case DbConnectionValidatorResult.DocumentFound:
-                    errors.Add(new ModelError(nameof(TenantViewModel.TablePrefix), S["The provided table prefix already exists."]));
+                case DbConnectionValidatorResult.DocumentTableFound:
+                    errors.Add(new ModelError(nameof(TenantViewModel.TablePrefix), S["The provided database and table prefix are already in use."]));
                     break;
             }
         }

src/OrchardCore/OrchardCore.Data.YesSql.Abstractions/DbConnectionValidatorResult.cs

@@ -1,22 +1,42 @@
 namespace OrchardCore.Data;
 
+/// <summary>
+/// The result from validating a database connection using <see cref="IDbConnectionValidator"/>.
+/// </summary>
 public enum DbConnectionValidatorResult
 {
-    // Unknown indicates that the connection string status is unknown or was not yet validated
+    /// <summary>
+    /// The connection string status is unknown or was not validated.
+    /// </summary>
     Unknown,
 
-    // NoProvider indicated that the provider is missing
+    /// <summary>
+    /// The database provider is missing.
+    /// </summary>
     NoProvider,
 
-    // DocumentNotFound indicates that the connection string was valid, yet the Document table does not exist 
-    DocumentNotFound,
+    /// <summary>
+    /// The connection string is valid and the 'Document' table does not exists.
+    /// </summary>
+    DocumentTableNotFound,
 
-    // DocumentFound indicates that the connection string was valid, yet the Document table exist
-    DocumentFound,
+    /// <summary>
+    /// The connection string is valid and the 'Document' table exists.
+    /// </summary>
+    DocumentTableFound,
 
-    // InvalidConnection unable to open a connection to the given connection string
+    /// <summary>
+    /// The 'Document' table exists with no 'ShellDescriptor' document.
+    /// </summary>
+    ShellDescriptorDocumentNotFound,
+
+    /// <summary>
+    /// Unable to open a connection with the given database connection string.
+    /// </summary>
     InvalidConnection,
 
-    // UnsupportedProvider indicated invalid or unsupported database provider
+    /// <summary>
+    /// Unsupported database provider.
+    /// </summary>
     UnsupportedProvider
 }

src/OrchardCore/OrchardCore.Data.YesSql.Abstractions/IDbConnectionValidator.cs

@@ -4,5 +4,5 @@ namespace OrchardCore.Data;
 
 public interface IDbConnectionValidator
 {
-    Task<DbConnectionValidatorResult> ValidateAsync(string databaseProvider, string connectionString, string tablePrefix);
+    Task<DbConnectionValidatorResult> ValidateAsync(string databaseProvider, string connectionString, string tablePrefix, bool isDefaultShell);
 }

src/OrchardCore/OrchardCore.Data.YesSql/DbConnectionValidator.cs

@@ -8,6 +8,7 @@
 using MySqlConnector;
 using Npgsql;
 using OrchardCore.Data.YesSql.Abstractions;
+using OrchardCore.Environment.Shell.Descriptor.Models;
 using YesSql;
 using YesSql.Provider.MySql;
 using YesSql.Provider.PostgreSql;
@@ -34,7 +35,7 @@ IOptions<YesSqlOptions> yesSqlOptions
         _yesSqlOptions = yesSqlOptions.Value;
     }
 
-    public async Task<DbConnectionValidatorResult> ValidateAsync(string databaseProvider, string connectionString, string tablePrefix)
+    public async Task<DbConnectionValidatorResult> ValidateAsync(string databaseProvider, string connectionString, string tablePrefix, bool isDefaultShell)
     {
         if (String.IsNullOrWhiteSpace(databaseProvider))
         {
@@ -50,7 +51,7 @@ public async Task<DbConnectionValidatorResult> ValidateAsync(string databaseProv
 
         if (provider != null && !provider.HasConnectionString)
         {
-            return DbConnectionValidatorResult.DocumentNotFound;
+            return DbConnectionValidatorResult.DocumentTableNotFound;
         }
 
         if (String.IsNullOrWhiteSpace(connectionString))
@@ -72,32 +73,72 @@ public async Task<DbConnectionValidatorResult> ValidateAsync(string databaseProv
         }
 
         var selectBuilder = GetSelectBuilderForDocumentTable(tablePrefix, providerName);
-
         try
         {
             var selectCommand = connection.CreateCommand();
             selectCommand.CommandText = selectBuilder.ToSqlString();
 
             using var result = await selectCommand.ExecuteReaderAsync();
-
-            // at this point the query succeeded and the table exists
-            return DbConnectionValidatorResult.DocumentFound;
+            if (!isDefaultShell)
+            {
+                // The 'Document' table exists.
+                return DbConnectionValidatorResult.DocumentTableFound;
+            }
+
+            var columns = Enumerable.Range(0, result.FieldCount).Select(result.GetName);
+            if (!columns.Any(c => c == "Type") || !columns.Any(c => c == "Content") || !columns.Any(c => c == "Version"))
+            {
+                // The 'Document' table exists with another schema.
+                return DbConnectionValidatorResult.DocumentTableFound;
+            }
         }
         catch
         {
-            // at this point we know that the document table does not exist
+            // The 'Document' table does not exist.
+            return DbConnectionValidatorResult.DocumentTableNotFound;
+        }
+
+        selectBuilder = GetSelectBuilderForShellDescriptorDocument(tablePrefix, providerName);
+        try
+        {
+            var selectCommand = connection.CreateCommand();
+            selectCommand.CommandText = selectBuilder.ToSqlString();
 
-            return DbConnectionValidatorResult.DocumentNotFound;
+            using var result = await selectCommand.ExecuteReaderAsync();
+            if (!result.HasRows)
+            {
+                // The 'Document' table exists with no 'ShellDescriptor' document.
+                return DbConnectionValidatorResult.ShellDescriptorDocumentNotFound;
+            }
+        }
+        catch
+        {
         }
+
+        // The 'Document' table exists.
+        return DbConnectionValidatorResult.DocumentTableFound;
     }
 
     private ISqlBuilder GetSelectBuilderForDocumentTable(string tablePrefix, DatabaseProviderName providerName)
     {
         var selectBuilder = GetSqlBuilder(providerName, tablePrefix);
 
         selectBuilder.Select();
-        selectBuilder.AddSelector("*");
+        selectBuilder.Selector("*");
+        selectBuilder.Table(_tableNameConvention.GetDocumentTable());
+        selectBuilder.Take("1");
+
+        return selectBuilder;
+    }
+
+    private ISqlBuilder GetSelectBuilderForShellDescriptorDocument(string tablePrefix, DatabaseProviderName providerName)
+    {
+        var selectBuilder = GetSqlBuilder(providerName, tablePrefix);
+
+        selectBuilder.Select();
+        selectBuilder.Selector("*");
         selectBuilder.Table(_tableNameConvention.GetDocumentTable());
+        selectBuilder.WhereAnd($"Type = '{typeof(ShellDescriptor).FullName}, {typeof(ShellDescriptor).Assembly.GetName().Name}'");
         selectBuilder.Take("1");
 
         return selectBuilder;
@@ -111,7 +152,7 @@ private static IConnectionFactory GetFactory(DatabaseProviderName providerName,
             DatabaseProviderName.MySql => new DbConnectionFactory<MySqlConnection>(connectionString),
             DatabaseProviderName.Sqlite => new DbConnectionFactory<SqliteConnection>(connectionString),
             DatabaseProviderName.Postgres => new DbConnectionFactory<NpgsqlConnection>(connectionString),
-            _ => throw new ArgumentOutOfRangeException("Unsupported Database Provider"),
+            _ => throw new ArgumentOutOfRangeException(nameof(providerName), "Unsupported database provider"),
         };
     }
 
@@ -123,7 +164,7 @@ private ISqlBuilder GetSqlBuilder(DatabaseProviderName providerName, string tabl
             DatabaseProviderName.MySql => new MySqlDialect(),
             DatabaseProviderName.Sqlite => new SqliteDialect(),
             DatabaseProviderName.Postgres => new PostgreSqlDialect(),
-            _ => throw new ArgumentOutOfRangeException("Unsupported Database Provider"),
+            _ => throw new ArgumentOutOfRangeException(nameof(providerName), "Unsupported database provider"),
         };
 
         var prefix = String.Empty;

src/OrchardCore/OrchardCore.Infrastructure/Shells.Database/Configuration/DatabaseShellsStorageOptions.cs

@@ -1,9 +1,11 @@
+using OrchardCore.Data;
+
 namespace OrchardCore.Shells.Database.Configuration
 {
     public class DatabaseShellsStorageOptions
     {
         public bool MigrateFromFiles { get; set; }
-        public string DatabaseProvider { get; set; }
+        public DatabaseProviderName DatabaseProvider { get; set; }
         public string ConnectionString { get; set; }
         public string TablePrefix { get; set; }
     }

src/OrchardCore/OrchardCore.Infrastructure/Shells.Database/Extensions/DatabaseShellContextFactoryExtensions.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Threading.Tasks;
+using OrchardCore.Data;
 using OrchardCore.Environment.Shell;
 using OrchardCore.Environment.Shell.Builders;
 using OrchardCore.Environment.Shell.Descriptor.Models;
@@ -12,7 +13,7 @@ public static class DatabaseShellContextFactoryExtensions
     {
         internal static Task<ShellContext> GetDatabaseContextAsync(this IShellContextFactory shellContextFactory, DatabaseShellsStorageOptions options)
         {
-            if (options.DatabaseProvider == null)
+            if (options.DatabaseProvider == DatabaseProviderName.None)
             {
                 throw new ArgumentNullException(nameof(options.DatabaseProvider),
                     "The 'OrchardCore.Shells.Database' configuration section should define a 'DatabaseProvider'");
@@ -24,7 +25,7 @@ internal static Task<ShellContext> GetDatabaseContextAsync(this IShellContextFac
                 State = TenantState.Running
             };
 
-            settings["DatabaseProvider"] = options.DatabaseProvider;
+            settings["DatabaseProvider"] = options.DatabaseProvider.ToString();
             settings["ConnectionString"] = options.ConnectionString;
             settings["TablePrefix"] = options.TablePrefix;
 

src/OrchardCore/OrchardCore.Setup.Core/SetupService.cs

@@ -170,7 +170,7 @@ private async Task<string> SetupInternalAsync(SetupContext context)
                 shellSettings["TablePrefix"] = context.Properties.TryGetValue(SetupConstants.DatabaseTablePrefix, out var databaseTablePrefix) ? databaseTablePrefix?.ToString() : String.Empty;
             }
 
-            switch (await _dbConnectionValidator.ValidateAsync(shellSettings["DatabaseProvider"], shellSettings["ConnectionString"], shellSettings["TablePrefix"]))
+            switch (await _dbConnectionValidator.ValidateAsync(shellSettings["DatabaseProvider"], shellSettings["ConnectionString"], shellSettings["TablePrefix"], shellSettings.IsDefaultShell()))
             {
                 case DbConnectionValidatorResult.NoProvider:
                     context.Errors.Add(String.Empty, S["DatabaseProvider setting is required."]);
@@ -181,8 +181,8 @@ private async Task<string> SetupInternalAsync(SetupContext context)
                 case DbConnectionValidatorResult.InvalidConnection:
                     context.Errors.Add(String.Empty, S["The provided connection string is invalid or server is unreachable."]);
                     break;
-                case DbConnectionValidatorResult.DocumentFound:
-                    context.Errors.Add(String.Empty, S["The provided database table is already in use."]);
+                case DbConnectionValidatorResult.DocumentTableFound:
+                    context.Errors.Add(String.Empty, S["The provided database and table prefix are already in use."]);
                     break;
             }
 

test/OrchardCore.Tests/Modules/OrchardCore.Tenants/Services/TenantValidatorTests.cs

@@ -122,7 +122,7 @@ private static TenantValidator CreateTenantValidator(bool defaultTenant = true)
                 : new ShellSettings();
 
             var connectionFactory = new Mock<IDbConnectionValidator>();
-            connectionFactory.Setup(l => l.ValidateAsync(shellSettings["ProviderName"], shellSettings["ConnectionName"], shellSettings["TablePrefix"]));
+            connectionFactory.Setup(l => l.ValidateAsync(shellSettings["ProviderName"], shellSettings["ConnectionName"], shellSettings["TablePrefix"], shellSettings.IsDefaultShell()));
 
             return new TenantValidator(
                 ShellHost,