Initial setup fails when Database Shells is active

tommi-saaskilahti commented 6 months ago

Describe the bug

Initial setup fails when Database Shells is active. This does not happen when DefaultIdentityColumnSize is set to Int32.

To Reproduce

Steps to reproduce the behavior:

Create database
Configure OrchardCore_Shells_Database in the appsettings.json
Run first time setup

Expected behavior

Database is expected to initialize correctly with new default setting for the identity column size.

Screenshots

tommi-saaskilahti commented 6 months ago

I forgot to mention, but this happens with SQL Server and Azure SQL. I'm not sure if this happens with MySQL or PostgresSQL.

sebastienros commented 6 months ago

Configure OrchardCore_Shells_Database in the appsettings.json

Can you explain what values you use for this?

tommi-saaskilahti commented 6 months ago

I apologize for the delayed response. We've been using values similar to these:

appsettings.json

{
  "UseSerilog": true,
  "Logging": {
    "LogLevel": {
      "Default": "Information",
      "Microsoft.AspNetCore": "Warning"
    }
  },
  "AllowedHosts": "example.com",
  "OrchardCore": {
    "OrchardCore_Shells_Database": {
      "DatabaseProvider": "SqlConnection",
      "ConnectionString": "Server=tcp:XXXXXXXXX.database.windows.net,1433;Initial Catalog=XXXXXXXXX-dev;Persist Security Info=False;User ID=XXXXXXXXX;Password=XXXXXXXXX;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;",
      "TablePrefix": "",
      "MigrateFromFiles": true
    },
    ...
  }
}

Or when database shells are configured with App Service environment variables:

OrchardCore__OrchardCore_Shells_Database__DatabaseProvider="SqlConnection"
OrchardCore__OrchardCore_Shells_Database__ConnectionString="Server=tcp:XXXXXXXXX.database.windows.net,1433;Initial Catalog=XXXXXXXXX-dev;Persist Security Info=False;User ID=XXXXXXXXX;Password=XXXXXXXXX;MultipleActiveResultSets=False;Encrypt=True;TrustServerCertificate=False;Connection Timeout=30;"

I don't know if the MigrateFromFiles option should be true or false during the initial setup with the database shell configuration. I have not encountered any problems previously with the value set to true, but the installation currently fails for both values.

During the setup, the [dbo].[Document] and [dbo].[Identifiers] tables are created successfully. However, when the migrations start executing, they all fail. After the setup, when the login view is opened, all login attempts fail due to the UserIndex table being missing.

sebastienros commented 6 months ago

Have you tried to change the settings to match the column type you are already using which is not the new default? Like this: https://github.com/OrchardCMS/OrchardCore/blob/main/src/OrchardCore.Cms.Web/appsettings.json#L26

Please see this changelog: https://docs.orchardcore.net/en/latest/docs/releases/1.7.0/

mvarblow commented 4 months ago

I recently reproduced this issue with the following using Orchard 1.8.3:

    "OrchardCore": {
        "ConnectionString": "...redacted",
        "DatabaseProvider": "SqlConnection",
        "DocumentTable": "Document",
        "IdentityColumnSize": "Int64",
        "RecipeName": "Config",
        "Schema": "orchard",
        "TableNameSeparator": "_",
        "OrchardCore_Shells_Database": {
            "ConnectionString": "...redacted",
            "DatabaseProvider": "SqlConnection",
            "DocumentTable": "Document",
            "IdentityColumnSize": "Int64",
            "Schema": "orchard",
            "TableNameSeparator": "_",
            "MigrateFromFiles": true
        }
    },

I started the site, submitted the setup page, and then got the following error (with many follow-on errors):

fail: OrchardCore.Data.Migration.DataMigrationManager[0]
      Error while running migration version 0 for 'OrchardCore.Users'.
Microsoft.Data.SqlClient.SqlException (0x80131904): Column 'orchard.Document.Id' is not the same data type as referencing column 'UserIndex.DocumentId' in foreign key 'FK_UserIndex'.
Could not create constraint or index. See previous errors.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, SqlCommand command, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, Boolean isInternal, String endMethod)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
   at Microsoft.Data.SqlClient.SqlCommand.<>c.<InternalExecuteNonQueryAsync>b__193_1(IAsyncResult asyncResult)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location ---
   at Dapper.SqlMapper.ExecuteImplAsync(IDbConnection cnn, CommandDefinition command, Object param) in /_/Dapper/SqlMapper.Async.cs:line 662
   at YesSql.Sql.SchemaBuilder.ExecuteAsync(IEnumerable`1 statements)
   at YesSql.Sql.SchemaBuilder.CreateForeignKeyAsync(String name, String srcTable, String[] srcColumns, String destTable, String[] destColumns)
   at YesSql.Sql.SchemaBuilder.CreateMapIndexTableAsync(Type indexType, Action`1 table, String collection)
   at OrchardCore.Users.Migrations.CreateAsync()
   at OrchardCore.Data.Migration.DataMigrationManager.UpdateAsync(String featureId)

But I get the same error if I change the IdentityColumnSize to Int32 (in both places). It seems that it just doesn't work to run the initial site setup with AddDatabaseShellsConfiguration in place?

mvarblow commented 4 months ago

A bit more information. Maybe this was already obvious from the original issue description, but this fails:

"OrchardCore": {
    "ConnectionString": "... redacted... ",
    "DatabaseProvider": "SqlConnection",
    "DocumentTable": "Document",
    "IdentityColumnSize": "Int64",
    "RecipeName": "Config",
    "Schema": "orchard",
    "TableNameSeparator": "_",
    "OrchardCore_Shells_Database": {
        "ConnectionString": "... redacted ...",
        "DatabaseProvider": "SqlConnection",
        "DocumentTable": "Document",
        "IdentityColumnSize": "Int64",
        "Schema": "orchard",
        "TableNameSeparator": "_",
        "MigrateFromFiles": true
    },
    "OrchardCore_Data_TableOptions": {
        "DefaultIdentityColumnSize": "Int64"
    }
},

and this works:

"OrchardCore": {
    "ConnectionString": "... redacted ...",
    "DatabaseProvider": "SqlConnection",
    "DocumentTable": "Document",
    "IdentityColumnSize": "Int32",
    "RecipeName": "Config",
    "Schema": "orchard",
    "TableNameSeparator": "_",
    "OrchardCore_Shells_Database": {
        "ConnectionString": "... redacted ...",
        "DatabaseProvider": "SqlConnection",
        "DocumentTable": "Document",
        "IdentityColumnSize": "Int32",
        "Schema": "orchard",
        "TableNameSeparator": "_",
        "MigrateFromFiles": true
    },
    "OrchardCore_Data_TableOptions": {
        "DefaultIdentityColumnSize": "Int32"
    }
},

I was trying this with OrchardCore 1.8.3 with the initial (single) tenant setup.

mvarblow commented 4 months ago

I did some debugging. The problem seems to be in ShellSettingsExtensions.GetIdentityColumnSize. If I set a breakpoint here and then start up a fresh OrchardCore tenant with the OrchardCore_Shells_Database settings and a call to AddDatabaseShellsConfiguration in my Program.cs...

I hit the breakpoints in GetIdentityColumnSize several times as Orchard Core starts and builds the initial setup page. Each time the breakpoint hits...

The above code block results in identityColumnSize of null. The shellSettings configuration is pretty empty at this point. It contains only ConnectionString, DatabaseProvider, Schema, and TablePrefix. If I add IdentityColumnSize to and/or DefaultIdentityColumnSize (in the appropriate section, as in my comment from yesterday) it doesn't matter. Those values do not appear in shellSettings. So GetIdentityColumnSize always falls into this if block:

And the first time through, as it's preparing the setup page, IsInitialized returns true (shellSettings.State is Running) every time. So identityColumnSize gets set to Int32. Once the setup page is displayed (before I complete and submit it), I can see that the Document and Identifiers tables both exist. Those are the only tables in the DB. The Document table has two rows. One for ShellSettings and one for ShellConfigurations. Both are pretty empty (just a VersionId and TenantId in the shell settings JSON). But they were both created with Int32 identifiers.

Now if I submit the setup form then the breakpoint in GetIdentityColumnSize hits again. But this time, two things have changed. The shellSettings.State is now Initializing. This would result in Int64 from the fallback logic:

But also, shellSettings now contains the values for OrchardCore_Data_TableOptions, including DefaultIdentityColumnSize, which is Int64. So now, GetIdentityColumnSize returns Int64. And Orchard Core gives the errors which were described in the comments above.

So, you can only use the database shells feature with Int32 columns. If you try to use it with Int64 (via the overrides or just by not specifying an override) you'll end up with a mix of Int32 (on the Document table) and Int64 identifier columns (on the rest) which does not work.

@sebastienros I'm wondering why GetIdentityColumnSize doesn't always fall back to Int64? Why does it use Int32 as the default if the shellSettings are initialized?

mvarblow commented 4 months ago

One more note from my debugging session... The shellSettings which are used early, when first displaying the setup page, are initialized by DatabaseShellContextFactoryExtensions. This is why it only has the four specific shell settings and nothing else and this is why the status is "Running":

I see no way to get either the default identity column size or the identity column size settings injected in here (to allow me to override the strange default that flips between Int32 before the setup service initializes the tenant and Int64 after).

mvarblow commented 4 months ago

I found a workaround of sorts. In my application, we're using a very limited set of OrchardCore modules. Instead of using AddOrchardCms in our Program.cs, we use AddOrchardCore. This allows me the flexibility to stick a Configure call onto the OrchardCoreBuilder right before I call AddDataAccess. I can use this to register an initializer that runs right before the initializer registered by AddDataAccess. Here I can hard code the identity column size to Int64.

builder.Services.AddOrchardCore()
    .AddCommands()
    .AddSecurity()
    .AddMvc()
    .AddIdGeneration()
    .AddEmailAddressValidator()
    .AddHtmlSanitizer()
    .AddSetupFeatures("OrchardCore.AutoSetup")
    .ConfigureServices(s =>
    {
        // Register an initializer to set the identity column size to Int64. This should be the default, but oddly, the default is
        // is Int32 briefly (just long enough to create the Document and Identifiers tables) before it changes to Int64 when the
        // setup service first initializes the tenant.
        s.Initialize(sp =>
        {
            IStore? store = sp.GetService<IStore>();
            if (store != null)
            {
                store.Configuration.IdentityColumnSize = IdentityColumnSize.Int64;
            }
            return new ValueTask();
        });
    })
    .AddDataAccess()
    .AddDataStorage()
    .AddBackgroundService()
    .AddScripting()
    .AddTheming()
    .AddCaching()
    .AddDatabaseShellsConfiguration()
    .ConfigureServices(s =>
    {
        s.AddResourceManagement();
        s.AddTagHelpers<LinkTagHelper>();
        s.AddTagHelpers<MetaTagHelper>();
        s.AddTagHelpers<ResourcesTagHelper>();
        s.AddTagHelpers<ScriptTagHelper>();
        s.AddTagHelpers<StyleTagHelper>();
    });

This seems like a pretty hacky workaround, but it's working for me. With this solution I don't need to override any of the configuration settings for index size.

(To be clear, I would have expected that I wouldn't need to override any of the settings since I want to use Int64 identity columns. I want to use Int64 identity columns specifically because they're supposed to be the default and I don't want to change the default. But, in fact, if I enable the database shells feature, I have to use this hack to use Int64 identity columns. If I wanted to use Int32 identity columns, then I could have used the normal settings-based mechanism to override the default.)

OrchardCMS / OrchardCore